Open nwalters512 opened 2 years ago
@chimurai 👋 can I do anything to help this along?
Sorry for the late reply. I'll need some time to review 🙏
@chimurai thanks!
@chimurai is there any chance you'd be able to review soon? If so, I'll resolve the merge conflicts with master.
Description
When there is an error from user-provided code (
pathFilter
,rewritePath
, orrouter
), that error is now handled and exposed to the user viaproxy.emit('error', ...)
.error-response-plugin
was also updated to handle the fact that it might receive a socket. If it does, it will callsocket.destroy()
.Motivation and Context
See #822 for a description of the issue being fixed. This PR closes #822.
How has this been tested?
I added a new test case. It fails on master, and works correctly with my new changes.
Types of changes
I'm split on whether or not this should be considered a breaking change. Folks could have written code assuming that the error handler would always get a
Response
as the third argument, whereas now it would get aSocket
. That said, the TypeScript types fromhttp-proxy
do reflect that that could be aSocket
, so it shouldn't come as too much of a surprise.Checklist: