Only chain on 404 errors, log and return others as 500 #802
Conversation
src/web/mod.rs
Outdated
| } else { | ||
| Err(e) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Suggestions on ways to rewrite this would be appreciated, seems like it needs some sort of conditional or_else_if(|e| e.response.status == Some(status::NotFound), |e| self.router_handler.handle(req)).
There was a problem hiding this comment.
I think a helper handle_if_404 closure would be nice.
There was a problem hiding this comment.
Something like
let handle_if_404 = |e, handler| if e.response.status == Some(status::NotFound) {
handler(req)
} else {
Err(e)
};
Nemo157
force-pushed
the
chain-404
branch
2 times, most recently
from
7775b99 to
438b9e9
Compare
| Page::new("internal server error".to_owned()) | ||
| .set_status(status::InternalServerError) | ||
| .title("Internal server error") | ||
| .to_resp("error") |
There was a problem hiding this comment.
Do we want to link to the GitHub page so people can make an issue?
There was a problem hiding this comment.
Might be a good followup issue to do once the template changes are in, all the error pages could do with a bit of a sprucing up.
There was a problem hiding this comment.
I don't think asking to open an issue on GitHub this way is much beneficial for us: without a backtrace it's hard to then debug those issues.
A thing we could do is generate a random UUID, print it in the same line as the error and show it in the error page, so when people open the issue we can simply do a journalctl -u docs.rs | grep UUID to get the exact error the user encountered.
There was a problem hiding this comment.
The UUID sounds useful but it can go in a follow-up, for now I want to get the bug fix in.
src/web/mod.rs
Outdated
| } else { | ||
| Err(e) | ||
| } | ||
| }) |
There was a problem hiding this comment.
I think a helper handle_if_404 closure would be nice.
| Page::new("internal server error".to_owned()) | ||
| .set_status(status::InternalServerError) | ||
| .title("Internal server error") | ||
| .to_resp("error") |
There was a problem hiding this comment.
The UUID sounds useful but it can go in a follow-up, for now I want to get the bug fix in.
Results in logs like
and a page like
when I reapply the bug in #728
Fixes #801