use sync method on Responder trait #1891
Conversation
fakeshadow
added
A-files
| /// Convert itself to `AsyncResult` or `Error`. | ||
| fn respond_to(self, req: &HttpRequest) -> Self::Future; | ||
| /// Convert self to `Response`. | ||
| fn respond_to(self, req: &HttpRequest) -> Response; |
There was a problem hiding this comment.
I think it would be better to have -> Result<Response, impl Into<Error>> so that pre-made responses from the error module can be used.
There was a problem hiding this comment.
After respond_to is called it's immediately matched by HandlerService and converted to Response anyway.
Return result here is kinda pointless by adding one additional branch and make the type larger by 8 bytes.
I believe a macro like this for error type would be a better alternative
macro_rules! err_respond {
($ty: ty) => (
impl Responder for $ty {
fn respond_to(self, _: &HttpRequest) -> Response {
Response::from_error(self.into())
}
}
)
}There was a problem hiding this comment.
hard no on the macro approach tbh
There was a problem hiding this comment.
impl Responder for error type is mostly not useful anyway. as impl T would still be one type making a handler can return error and only error. It's part of the reason why not all crate error types impl Responder
Biggest change is Responder is no longer async: actix/actix-web#1891 Left a TODO for testing load_stream, as well.
Biggest change is Responder is no longer async: actix/actix-web#1891 Left a TODO for testing load_stream, as well. Signed-off-by: Jim Crossley <jim@crossleys.org>
Biggest change is Responder is no longer async: actix/actix-web#1891 Left a TODO for testing load_stream, as well. Signed-off-by: Jim Crossley <jim@crossleys.org>
Biggest change is Responder is no longer async: actix/actix-web#1891 Signed-off-by: Jim Crossley <jim@crossleys.org>
Biggest change is Responder is no longer async: actix/actix-web#1891 Signed-off-by: Jim Crossley <jim@crossleys.org>
PR Type
Refactor
PR Checklist
Check your PR fulfills the following:
Overview
Through out
actix-webactix-extrasandexamplesrepo there isn't a single instance whereResponderreturns a truely future type. They are all types wrapped in future types for an illusion of async.This adds a little bit overhead(almost non measurable though) and complicated
Respondertrait with two associated types and future wrappers to deal with the difference of various future types.This PR address this by making the trait sync and returns only
Response<Body>type. It makes the trait more simple and performant(in theory).This is a breaking change for people using futures inside custom
Respondertrait but it would be fairly easy to move these async code into handler function where async code should be ran.