feat: add forwardError option to enable error forwarding to next middleware#2259
feat: add forwardError option to enable error forwarding to next middleware#2259bjohansebas merged 9 commits intonextfrom
forwardError option to enable error forwarding to next middleware#2259Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #2259 +/- ##
==========================================
+ Coverage 95.67% 96.92% +1.25%
==========================================
Files 13 13
Lines 901 911 +10
Branches 263 266 +3
==========================================
+ Hits 862 883 +21
+ Misses 39 28 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
alexander-akait
left a comment
alexander-akait
left a comment
There was a problem hiding this comment.
Yeah, not ever frameworks supports it, so we should add this option to docs and write which frameworks support it
alexander-akait
left a comment
alexander-akait
left a comment
There was a problem hiding this comment.
Looks good, we can merge this, also let's add this in our doc
|
@bjohansebas Let's finish this todo, because it is easy, but for |
…r middleware setup
…th forwardError option
|
Let's fix lint, some test failed, but I think it is due hono, one test is unstable |
| "forwardError": { | ||
| "description": "Enable or disable forwarding errors to next middleware.", | ||
| "link": "https://github.com/webpack/webpack-dev-middleware#forwarderrors", | ||
| "type": "boolean" |
There was a problem hiding this comment.
Let's add this to our README too
| }); | ||
|
|
||
| // TODO: why koa and hono don't catch for their error handling when stream emit error? | ||
| (name === "koa" || name === "hono" ? it.skip : it)( |
There was a problem hiding this comment.
The behavior here in Hono and Koa is a bit strange, and it has to do with how they handle streams when sending the response. If an error is thrown, it’s not really captured, in fact, Hono would treat it as if forwardError were false.
There was a problem hiding this comment.
Yeah, I know it, but let's resolve it later, not critical for us
There was a problem hiding this comment.
So do you want me to resolve it in another PR?
| }); | ||
|
|
||
| // TODO: why koa and hono don't catch for their error handling when stream emit error? | ||
| (name === "koa" || name === "hono" ? it.skip : it)( |
There was a problem hiding this comment.
So do you want me to resolve it in another PR?
|
|
||
| Enable or disable forwarding errors to the next middleware. If `true`, errors will be forwarded to the next middleware, otherwise, they will be handled by `webpack-dev-middleware` and a response will be handled case by case. | ||
|
|
||
| This option don't work with hono, koa and hapi, because of the differences in error handling between these frameworks and express. |
There was a problem hiding this comment.
It's partially true, there are certain things that don’t work, but I think it’s better to say it doesn’t work first, and then once we know it works correctly, say that it does.
bjohansebas
changed the title
forwardError option to enable error forwarding to next middlewareforwardError option to enable error forwarding to next middleware
Yeah, it is in my TODO, we have the same problem with the one test (random fails) |
2 participants
Summary
While it works fine in Express, I’m not sure how it will behave in other frameworks, so testing is in progress.
What kind of change does this PR introduce?
Did you add tests for your changes?
Does this PR introduce a breaking change?
If relevant, what needs to be documented once your changes are merged or what have you already documented?