-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix(proxy): handle error when proxy itself errors #13929
Conversation
|
e794f69
to
befb2af
Compare
@@ -51,6 +51,10 @@ export function proxyMiddleware( | |||
} | |||
|
|||
proxy.on('error', (err, req, originalRes) => { | |||
// originalRes can be falsy if the proxy itself errored | |||
if (!originalRes) { | |||
httpServer?.emit('error', err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since httpServer
can be null
(For example, if Vite is running in middleware mode), I think it would be better to output the error here directly (like we do below).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing it out!
I will change it right away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM
Description
closes #13703
Add error handling to the point when the proxy itself errors.
Right now, the
proxy
expects that theres
is neverundefined
or other falsy value but there are some cases when theres
is.When the
res
is falsy, the thrown error becomes the error of accessingundefined
value, not the original error.This makes it hard for developers to trace the original error.
Therefore, I added a logic to handle error case when the
res
is falsy.Additional context
My particular case of the problem was passing
server.proxy
'starget
as falsy value (null
orundefined
).AS-IS
Just seeing the error above, it is hard to track where the error started from.
TO-BE
The above error can give developers more specific information about where the error started from.
I believe that my fix can cover so much more errors. Due to the fact that it is caused by an incorrect expectation of the type of
res
.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).