Convert internal X11 errors to anyhow. (#982)

Author: jneem

CHANGELOG.md

@@ -144,6 +144,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i
 - Refactored DPI scaling. ([#904] by [@xStrom])
 - Added docs generation testing for all features. ([#942] by [@xStrom])
 - Renamed `BaseState` to `WidgetState` ([#969] by [@cmyr])
+- X11: Reworked error handling ([#982] by [@jneem])
 
 ### Outside News
 
@@ -229,6 +230,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i
 [#969]: https://github.com/xi-editor/druid/pull/969
 [#970]: https://github.com/xi-editor/druid/pull/970
 [#980]: https://github.com/xi-editor/druid/pull/980
+[#982]: https://github.com/xi-editor/druid/pull/982
 
 ## [0.5.0] - 2020-04-01
 

Cargo.lock

@@ -26,6 +26,7 @@ lazy_static = "1.0"
 time = "0.2.7"
 cfg-if = "0.1.10"
 instant = { version = "0.1", features = ["wasm-bindgen"] }
+anyhow = "1.0.31"
 
 # Optional dependencies
 cairo-rs = { version = "0.8.1", default_features = false, optional = true }

druid-shell/Cargo.toml

@@ -79,10 +79,7 @@ impl Application {
             return Err(Error::ApplicationAlreadyExists);
         }
         util::claim_main_thread();
-        let platform_app = match platform::Application::new() {
-            Ok(app) => app,
-            Err(err) => return Err(Error::Platform(err)),
-        };
+        let platform_app = platform::Application::new()?;
         let state = Rc::new(RefCell::new(State { running: false }));
         let app = Application {
             platform_app,

druid-shell/src/application.rs

@@ -15,6 +15,7 @@
 //! Errors at the application shell level.
 
 use std::fmt;
+use std::sync::Arc;
 
 use crate::platform::error as platform;
 
@@ -25,12 +26,10 @@ pub enum Error {
     ApplicationAlreadyExists,
     /// The window has already been destroyed.
     WindowDropped,
-    /// Runtime borrow failure.
-    BorrowError(BorrowError),
     /// Platform specific error.
     Platform(platform::Error),
     /// Other miscellaneous error.
-    Other(&'static str),
+    Other(Arc<anyhow::Error>),
 }
 
 impl fmt::Display for Error {
@@ -39,64 +38,23 @@ impl fmt::Display for Error {
             Error::ApplicationAlreadyExists => {
                 write!(f, "An application instance has already been created.")
             }
-            Error::WindowDropped => write!(f, "The window has already been destroyed."),
-            Error::BorrowError(err) => fmt::Display::fmt(err, f),
             Error::Platform(err) => fmt::Display::fmt(err, f),
+            Error::WindowDropped => write!(f, "The window has already been destroyed."),
             Error::Other(s) => write!(f, "{}", s),
         }
     }
 }
 
 impl std::error::Error for Error {}
 
-impl From<platform::Error> for Error {
-    fn from(src: platform::Error) -> Error {
-        Error::Platform(src)
+impl From<anyhow::Error> for Error {
+    fn from(src: anyhow::Error) -> Error {
+        Error::Other(Arc::new(src))
     }
 }
 
-/// Runtime borrow failure.
-#[derive(Debug, Clone)]
-pub struct BorrowError {
-    location: &'static str,
-    target: &'static str,
-    mutable: bool,
-}
-
-impl BorrowError {
-    pub fn new(location: &'static str, target: &'static str, mutable: bool) -> BorrowError {
-        BorrowError {
-            location,
-            target,
-            mutable,
-        }
-    }
-}
-
-impl fmt::Display for BorrowError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        if self.mutable {
-            // Mutable borrow fails when any borrow exists
-            write!(
-                f,
-                "{} was already borrowed in {}",
-                self.target, self.location
-            )
-        } else {
-            // Regular borrow fails when a mutable borrow exists
-            write!(
-                f,
-                "{} was already mutably borrowed in {}",
-                self.target, self.location
-            )
-        }
-    }
-}
-
-impl std::error::Error for BorrowError {}
-
-impl From<BorrowError> for Error {
-    fn from(src: BorrowError) -> Error {
-        Error::BorrowError(src)
+impl From<platform::Error> for Error {
+    fn from(src: platform::Error) -> Error {
+        Error::Platform(src)
     }
 }

druid-shell/src/error.rs

@@ -24,6 +24,9 @@
 pub use kurbo;
 pub use piet_common as piet;
 
+#[macro_use]
+mod util;
+
 mod application;
 mod clipboard;
 mod common_util;
@@ -36,7 +39,6 @@ mod menu;
 mod mouse;
 mod platform;
 mod scale;
-mod util;
 mod window;
 
 pub use application::{AppHandler, Application};

druid-shell/src/lib.rs

@@ -16,6 +16,7 @@
 
 use std::ffi::OsString;
 
+use anyhow::anyhow;
 use gtk::{FileChooserAction, FileChooserExt, FileFilter, NativeDialogExt, ResponseType, Window};
 
 use crate::dialog::{FileDialogOptions, FileDialogType, FileSpec};
@@ -89,18 +90,18 @@ pub(crate) fn get_file_dialog_path(
     let result = match result {
         ResponseType::Accept => match dialog.get_filename() {
             Some(path) => Ok(path.into_os_string()),
-            None => Err(Error::Other("No path received for filename")),
+            None => Err(anyhow!("No path received for filename")),
         },
-        ResponseType::Cancel => Err(Error::Other("Dialog was deleted")),
+        ResponseType::Cancel => Err(anyhow!("Dialog was deleted")),
         _ => {
             log::warn!("Unhandled dialog result: {:?}", result);
-            Err(Error::Other("Unhandled dialog result"))
+            Err(anyhow!("Unhandled dialog result"))
         }
     };
 
     // TODO properly handle errors into the Error type
 
     dialog.destroy();
 
-    result
+    Ok(result?)
 }

druid-shell/src/platform/gtk/dialog.rs

@@ -25,6 +25,7 @@ use std::slice;
 use std::sync::{Arc, Mutex, Weak};
 use std::time::Instant;
 
+use anyhow::anyhow;
 use gdk::{EventKey, EventMask, ModifierType, ScrollDirection, WindowExt};
 use gio::ApplicationExt;
 use gtk::prelude::*;
@@ -694,9 +695,7 @@ impl WindowHandle {
         if let Some(state) = self.state.upgrade() {
             dialog::get_file_dialog_path(state.window.upcast_ref(), ty, options)
         } else {
-            Err(ShellError::Other(
-                "Cannot upgrade state from weak pointer to arc",
-            ))
+            Err(anyhow!("Cannot upgrade state from weak pointer to arc").into())
         }
     }
 }