-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pre:error event in createApp #2036
Comments
This one I'm having a hard time understanding! For sure I'm missing context or knowledge :-) |
Errr... Nope, I can't grok it either. |
@JedWatson is this still a feature request to consider? Would this be an error handler that runs only in the userland app, or also the admin app? |
not sure if this is still relevant, closing as no activity for long. |
cc @snowkeeper, @creynders
There's currently no way to hook your own error handling middleware into the express app if you're using redirects (e.g. for custom 404 pages). I'm going to add a
pre:error
event to handle this.Not 100% sure how best to hook in an actual custom error handling method though (i.e. one that takes 4 arguments:
err, req, res, next
) - this has special behaviour in express and isn't the same as a 404 handler.Maybe the best way would be to add both
pre:notfound
andpre:error
events, but in that case the "pre" doesn't actually make a lot of sense unless you read it as "run this before the generic handlers for notfound / error conditions"I'm also a bit skeptical about the value of taking this much control over the express stack by default... it's really not that hard to configure in your project anyway, and the only reason not to just add these to the end of your
routes(app)
handler would be if you're using the built-in redirect functionality. Maybe a better answer is to back-track a bit here.Feedback welcome.
The text was updated successfully, but these errors were encountered: