feat: create router.error method to allow explicit definition of error-handling middleware#160
Open
nathanhleung wants to merge 2 commits intopillarjs:masterfrom
Open
feat: create router.error method to allow explicit definition of error-handling middleware#160nathanhleung wants to merge 2 commits intopillarjs:masterfrom
router.error method to allow explicit definition of error-handling middleware#160nathanhleung wants to merge 2 commits intopillarjs:masterfrom
Conversation
Author
|
Friendly ping @UlisesGascon @wesleytodd @bjohansebas — anything I can help out with here in terms of being able to get this merged? Thanks for your work as maintainers 🙏 |
Author
|
Thanks for running CI — seems like the failing test is unrelated to this PR? (Seeing same failure on #161 in https://github.com/pillarjs/router/actions/runs/15097652965/job/42434464916?pr=161) |
Member
|
yes, see #162 |
bea6fa5 to
c9ed239
Compare
Author
|
@UlisesGascon @wesleytodd @bjohansebas thanks for taking a look at #169. now that it's closed + unblocking this, could we run the workflows here? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues/PRs: expressjs/express#2896, #59
Summary
This PR creates a
router.errormethod which enables the explicit definition of error-handling middleware (first proposed by @feross ~10 years ago in expressjs/express#2896) as opposed to implicit definition by passing an arity-4 function torouter.use.The new
router.errormethod supports error handling middleware defined with either 3 or 4 arguments. The first argument to the middleware will always be the errorerr(thus avoiding the arity gotcha withrouter.use).The PR is backwards-compatible and makes no functional changes to any other
routermethods, includingrouter.use; it is purely additive. All tests pass locally for me, at least (macOS Sequoia, Node v22).Let me know your thoughts/feedback on the current approach and can revise.
Changes
The diff is ~80 source lines (plus ~40 lines of doc comments) and ~600 test lines. The new
router.errormethod mostly falls back on the existing logic inrouter.usewhen possible. Effectively, it wraps its argument in another wrapper function with arity 4 to trigger the existing error handling behavior inuse, and then the wrapped function is passed touse. Since it falls back on the existing logic inuse, the same path behavior also works (i.e.,errorcan be set to work with regular paths, parametrized paths, regexes, etc.)The PR differs from #59 in that the changes are isolated to the router itself, rather than being on the individual route objects.
Demo
Live demo on CodeSandbox (using my forked version of
router): https://codesandbox.io/p/devbox/y63xfg?file=%2Findex.js