Convert `Exception` to `Error`. #113

richard-uk1 · 2019-11-24T21:40:39Z

And use anyhow::Error rather than Box<dyn Error+...>.

fixes #86 .

And use `anyhow::Error` rather than `Box<dyn Error+...>`.

ghost · 2019-11-24T22:07:53Z

What is the reason for re-exporting the error type from anyhow rather than wrapping it in a new type?

dtolnay · 2019-11-25T00:36:14Z

Ditto -- you want this wrapped in your own type.

richard-uk1 · 2019-11-26T12:07:19Z

@dtolnay do you mean like this?

richard-uk1 · 2019-11-26T12:41:19Z

Since a boxed trait object is 'static by default, I could remove all the annotations in examples, if that was wanted.

yoshuawuyts · 2019-11-27T12:29:01Z

@derekdreery replacing the custom error types in the examples with this new error type would be v. welcome!

richard-uk1 · 2019-11-27T13:44:46Z

@yoshuawuyts

replacing the custom error types in the examples with this new error type would be v. welcome!

I can do this, but I would argue that we don't need to. The user of this crate (e.g. the examples) can use whatever error system they like - they are not tied to using surf::Error, and IMO it's good to demonstrate this in the examples.

ghost · 2019-11-27T14:54:03Z

src/error.rs

+
+impl Error {
+    /// Use this when you need to implement middleware, where the error type must be `surf::Error`.
+    pub fn new<E>(error: E) -> Self


Let's make this constructor private or pub(crate).

I originally did this, but I found that the user needed to be able to create surf::Errors in the middleware examples. I'll have a look to see if this can be avoided.

ghost · 2019-11-27T14:54:43Z

src/error.rs

+    }
+}
+
+impl Deref for Error {


It's not idiomatic to implement Deref and DerefMut on error types - only smart pointers should implement those traits.

ghost · 2019-11-27T14:57:46Z

In the examples, this is a little bit verbose:

async fn post_json() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
    // ...
}

We could simplify with:

type Error = Box<dyn std::error::Error + Send + Sync>;

async fn post_json() -> Result<(), Error> {
    // ...
}

Btw, I think it'd also be nice to have this in the crate so that surf::Result<T> becomes a thing :)

type Result<T> = std::result::Result<T, crate::Error>;

richard-uk1 · 2019-11-27T16:25:05Z

I've been looking at using a boxed error in middleware rather than surf::Error, but I run into a problem converting Box<dyn std::error::Error + Send + Sync> into a anyhow::Error. Is this possible? I would imagine not, since trait object representation is not stable (e.g. see rust-lang/rfcs#2594 for recent work).

Otherwise the options are to make Middleware take an Error assoicated type, or to allow users to create surf::Errors.

Other suggestions welcome!

dtolnay · 2019-11-27T23:02:06Z

but I run into a problem converting Box<dyn std::error::Error + Send + Sync> into a anyhow::Error. Is this possible? I would imagine not

anyhow! will do that conversion.

use anyhow::{anyhow, Result};

fn f() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
    Err(Box::from("oh no!"))
}

fn main() -> Result<()> {
    match f() {
        Err(err) => Err(anyhow!(err)),
        Ok(_) => unreachable!(),
    }
}

richard-uk1 · 2019-11-28T23:31:23Z

Cool! I've included that, so that the middleware can take any boxed error. It could still be switched to an associated type if people think that would be better. I've used an alias for boxed errors where it's hidden from the user to make the code less verbose, but where the user sees it I've always used the original type rather than an alias.

yoshuawuyts

It seems this patch is now defining custom BoxErrors in all examples; I'd like us to use surf::Error or surf::Result instead. This way people can learn about Surf's Error type.

Also CI failing because of missing cargo fmt. Would you mind running that? Thanks heaps for the work you've put in!

yoshuawuyts · 2019-11-27T17:37:53Z

src/error.rs

@@ -0,0 +1,59 @@
+
+use std::{fmt::{self, Display, Debug}, ops::{Deref, DerefMut}};


Could this not be nested. Thanks!

yoshuawuyts · 2019-11-27T17:38:20Z

src/error.rs

+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        self.0.source()
+    }
+}


I think we're missing From<Error> impls here also.

Should there be From<Error> impls for the log errors and the io errors in the next_reuse example?

yoshuawuyts · 2019-12-13T13:41:11Z

examples/hello_world.rs

 // The need for Ok with turbofish is explained here
 // https://rust-lang.github.io/async-book/07_workarounds/03_err_in_async_blocks.html
-fn main() -> Result<(), surf::Exception> {
+fn main() -> Result<(), BoxError> {


Could we use surf::Error here?

yoshuawuyts · 2019-12-13T13:41:20Z

README.md

@@ -118,7 +118,7 @@ And even creating streaming proxies is no trouble at all.
 ```rust
 use async_std::task;

-fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
+fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {


Could we use surf::Error here?

yoshuawuyts · 2019-12-13T13:41:46Z

examples/middleware.rs

 impl<C: HttpClient> Middleware<C> for Printer {
    fn handle<'a>(
        &'a self,
        req: Request,
        client: C,
        next: Next<'a, C>,
-    ) -> BoxFuture<'a, Result<Response, surf::Exception>> {
+    ) -> BoxFuture<'a, Result<Response, BoxError>> {


Could we use surf::Error here?

yoshuawuyts · 2019-12-13T13:41:55Z

examples/next_reuse.rs

 impl<C: HttpClient> Middleware<C> for Doubler {
    fn handle<'a>(
        &'a self,
        req: Request,
        client: C,
        next: Next<'a, C>,
-    ) -> BoxFuture<'a, Result<Response, surf::Exception>> {
+    ) -> BoxFuture<'a, Result<Response, BoxError>> {


Could we use surf::Error here?

richard-uk1 · 2020-02-17T17:12:44Z

This is out of date so I'm gonna close.

Convert Exception to Error.

8e55663

And use `anyhow::Error` rather than `Box<dyn Error+...>`.

Use opaque error type

e2c868a

richard-uk1 added 2 commits November 26, 2019 12:08

Merge remote-tracking branch 'upstream/master' into anyhow

3597952

Merge changes, make work.

9428068

ghost reviewed Nov 27, 2019

View reviewed changes

Follow recommendations in feedback

4553f79

richard-uk1 added 2 commits November 28, 2019 23:33

Missed a Box<dyn ...>

bdd08d2

Make all examples use generic error.

7bbf429

richard-uk1 force-pushed the anyhow branch from 4afbeae to 7bbf429 Compare November 28, 2019 23:39

yoshuawuyts requested changes Dec 13, 2019

View reviewed changes

aubaugh mentioned this pull request Dec 24, 2019

Convert Exception to Error cont. #133

Closed

yoshuawuyts mentioned this pull request Jan 19, 2020

Error patches http-rs/http-types#63

Merged

4 tasks

richard-uk1 closed this Feb 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert `Exception` to `Error`. #113

Convert `Exception` to `Error`. #113

richard-uk1 commented Nov 24, 2019

ghost commented Nov 24, 2019

dtolnay commented Nov 25, 2019

richard-uk1 commented Nov 26, 2019

richard-uk1 commented Nov 26, 2019

yoshuawuyts commented Nov 27, 2019

richard-uk1 commented Nov 27, 2019 •

edited

Loading

ghost Nov 27, 2019

richard-uk1 Nov 27, 2019

ghost Nov 27, 2019

ghost commented Nov 27, 2019 •

edited by ghost

Loading

richard-uk1 commented Nov 27, 2019

dtolnay commented Nov 27, 2019

richard-uk1 commented Nov 28, 2019

yoshuawuyts left a comment

yoshuawuyts Nov 27, 2019

yoshuawuyts Nov 27, 2019

aubaugh Dec 24, 2019

yoshuawuyts Dec 13, 2019

yoshuawuyts Dec 13, 2019

yoshuawuyts Dec 13, 2019

yoshuawuyts Dec 13, 2019

richard-uk1 commented Feb 17, 2020

		@@ -0,0 +1,59 @@

		use std::{fmt::{self, Display, Debug}, ops::{Deref, DerefMut}};

Convert Exception to Error. #113

Convert Exception to Error. #113

Conversation

richard-uk1 commented Nov 24, 2019

ghost commented Nov 24, 2019

dtolnay commented Nov 25, 2019

richard-uk1 commented Nov 26, 2019

richard-uk1 commented Nov 26, 2019

yoshuawuyts commented Nov 27, 2019

richard-uk1 commented Nov 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Nov 27, 2019 • edited by ghost Loading

richard-uk1 commented Nov 27, 2019

dtolnay commented Nov 27, 2019

richard-uk1 commented Nov 28, 2019

yoshuawuyts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richard-uk1 commented Feb 17, 2020

Convert `Exception` to `Error`. #113

Convert `Exception` to `Error`. #113

richard-uk1 commented Nov 27, 2019 •

edited

Loading

ghost commented Nov 27, 2019 •

edited by ghost

Loading