Error: additional response data

[Fishrock123]

Related to all of the following discussions, it would be useful for http_types's errors to optionally carry data similar to a response, particularly Headers and/or a Body.

Commit 1 allows for better handling of situations where things like middleware want to act only partially on an error.

Commit 2 allows for robust creation of Response-s directly from Errors, with error storage and retrieval.
(Includes a feature=unstable storage of the error and way to retrieve it later.)

Refs: http-rs/tide#546
Refs: http-rs/tide#532
Refs: http-rs/tide#452

[Fishrock123]

Wanted to make this as a draft... oh well. There are some XXX and TODO in the code...

[Fishrock123]

Unfortunately I'm getting a version/abi conflict when trying to upgrade just tide to this, but it should allow the following patch in tide's server.rs:

diff --git a/src/server.rs b/src/server.rs
index d0bff9d..b407f1b 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -408,15 +408,7 @@ impl<State: Send + Sync + 'static> Server<State> {
                 Ok(res.into())
             }
             Err(err) => {
-                let mut res = http_types::Response::new(err.status());
-                res.set_content_type(http_types::mime::PLAIN);
-                // Only send the message if it is a non-500 range error. All
-                // errors default to 500 by default, so sending the error
-                // body is opt-in at the call site.
-                if !res.status().is_server_error() {
-                    res.set_body(err.to_string());
-                }
-                Ok(res.into())
+                Ok(http_types::Response::from_error(err))
             }
         }
     }

And in turn would also remove the need/desire for this: https://github.com/http-rs/tide/pull/546/files#diff-3dbd16223f25e8b5e0bcf099743107e9R13

[jbr]

This part confuses me. If you can losslessly turn an Error into a Response, why do Errors need to be "response-like" by having headers and body? It seems like it would be preferable just to transform them into Responses instead of duplicating header and body logic on Error

[Fishrock123]

I'm not sure what you mean - in order to "losslessly" turn an Error into a Response, you'd need this information. The Response.error portion of this PR is secondary, the main idea is to be able to propagate an error as a distinct Error with enough information to make a Response back to any potential client once any middleware is finished with it.

[jbr]

I like the idea of Response having an Error, but have concerns about Error becoming any more Response-like. The duplication of API surface area introduces additional maintenance burden and potential mismatches between Response and Error.

[Fishrock123]

I don't disagree with the API surface, but the alternatives to this seem to be:

1. Put all error info onto Response (i.e. do away with Result). Very un-rust-like.
2. Always have a Response attached to an error. I am not sure that is any better though.

[jbr]

I do really like the idea of having an Option<Error> on Response, but I think it would be nice to explore patterns with only that change. In particular, I imagine it being used as follows:

I don't think Errors should have a way to add headers or body. If people want to add headers or a body to an Error variant, they should wrap it with a Response that holds the Error. Later error-handling middlewares can check if the Response has an attached Error and populate the body on the Response from the error, or modify headers, etc. If people want to specify this information in a route handler, they should just build a Response directly. Using ? and Errors should be a shortcut for passing the error along to a later middleware, and most of the time by the time an error reaches the end of the middleware stack, it should have been transformed into a Response. Once a Response had a source error, there would be no reason to remove that error or use it as anything other than a reference, since the only use for that error would be to modify the Response that holds it.

Is there some use case that is unhandled by that approach that requires Error to be more Response-like?

---

Additionally, I'm not convinced that any of this should exist in http-types. It seems like this could be primarily a tide concern, by adding Option<Error> to the tide::Response as a sibling to the inner http_types::Response

[Fishrock123]

Closing in favor of #174 as per this discussion: https://discordapp.com/channels/598880689856970762/649056551835009097/718227631421784084