Async middleware #148
Async middleware #148
Conversation
|
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 80 78 -2
=========================================
- Hits 80 78 -2
Continue to review full report at Codecov.
|
test/index.test.js
Outdated
| @@ -39,20 +39,21 @@ describe("nc()", () => { | |||
| .expect("quick math"); | |||
| }); | |||
|
|
|||
| it("supports async handlers", async () => { | |||
| it("supports async handlers and middleware", async () => { | |||
There was a problem hiding this comment.
Can you add them as new test cases instead of editing existed ones?
|
Hey! Just bumping this PR in case you missed it. Requested changes have been made! |
| }; | ||
| const next = (err) => i < len | ||
| ? err | ||
| ? onError(err, req, res, next) |
There was a problem hiding this comment.
What would be its behavior when an error is thrown. In this case the execution inside the handler still goes on after await next()?
There was a problem hiding this comment.
Yeah that's something I totally missed in my testing. my bad! Indeed errors aren't halting execution after await next().
It seems it's behaving this way because the error is handled inside of next() and never returned back. I found a solution that ensures that errors are propagated, but I can't find one that gives onError() an option to call next() and allow middleware to continue execution after doing so. Any ideas?
There was a problem hiding this comment.
We may need to decide on a behavior.
One thing we can do is to let next() throws and the error should propagate all the way up and only attach the error handler to the first handler:
handler1: await() -- onError()
middleware1: await next() -- ⬆️
middleware2: await next() - ⬆️
middleware3: throw new Error()
But that is a breaking change since it will break if user have not been doing await next(), which is probably the case now.
There was a problem hiding this comment.
My pending changes resolves error handling without having to unravel the stack. In the event that the error handler wishes to ignore the error and call next(), however, it leads to the following unexpected behaviour.
// ❌ = does not execute
// Middleware listed after .get() represents code to be executed after await next().
// Example: GET /
middleware1: await next()
middleware2: await next()
middleware3: throw new Error() -> onError() -> next()
.get(handler):
middleware3: ❌ -- not expected to run, it gave up control after throwing
middleware2: ❌ -- expected to run
middleare1: ❌ -- expected to run
This is where I'm running into problems with finding a decent solution. Do you think it's worth preserving this trait of error handling?
closes #146