actix / actix-web

Actix Web is a powerful, pragmatic, and extremely fast web framework for Rust.
https://actix.rs
Apache License 2.0
21.38k stars 1.66k forks source link

Allow access to request when rendering errors #2925

Open tp971 opened 1 year ago

tp971 commented 1 year ago

This was requested a few times already (the most recent one I found is #1159) but I want to elaborate a little more on this issue. I started using actix-web for web-development and plan to use it for production purposes. I wrote a project that uses actix-web and a template engine to render HTML pages. This means that I also want to render errors as HTML pages using the template engine, therefore I need access to the request for various things in the application state, including the templates.

I thought of various ways to implement this:

  1. Implement ResponseError on an error type and render the error in ResponseError::error_response. This would be the most ergonomic approach by far if the method received a HttpRequest, which it does not.
  2. Include the HttpRequest in the error type and then implement 1. This method works, but results in a lot of map_errs in the handler functions, making this approach more unergonomic.
  3. "Improve" 2. by writing a function f_inner for each handler function f, that returns an immediate error type (without the request) and only call map_err once in f to map to the full error type (with the request). While this reduces the amount of map_errs, it duplicates a lot of handler functions signatures (and in my opinion, mixing 2 and 3 makes the code inconsistent and even more unreadable). (This whole approach could be improved with try-block syntax, which is not stable yet)
  4. Writing a middleware that checks for an error and renders the template. This approach is even less idiomatic than the ones above because it potentially requires downcasting the error type for getting the original error type and requires much more insight into the actix-web library than the other approaches. Trying out this method I also had some issues with logging, e.g. that errors were not logged anymore or that the wrong status code was logged.

In my opinion, not being able to implement the first approach does not only harm the ergonomics of the code, it also might discourage people (especially more inexperienced Rust developers) because finding a good alternative to 1. seems unintuitive right now.

Of course changing the method signature, as suggested in previous issues, breaks the API and seemingly causes a few more complications. I think a better approach would be something like: allow handlers to return a Result<T, E> where T and E both implement Responder (which is also the approach used in the web-frameworks rocket and axum).

While I don't know whether this can be included in a non-breaking way, I still think it's worth to consider, maybe even for a breaking change going into actix v5.

rakbladsvalsen commented 1 year ago

Are there any news on this? I just ran into this issue, and logging is kinda broken when working with multiple middlewares that can fail.

wyatt-herkamp commented 1 month ago

I would like to see this too. The reason being is I would like to check to see if they have an Accept header. If it does and it equals Json return json otherwise return an html page.