Restore the response event. #31
Conversation
| if (options.userCallback) { | ||
| requestProxy.on('response', options.userCallback); | ||
| } | ||
| // forward all request events on the proxy except `response` |
There was a problem hiding this comment.
An alternative would be to forward no events, except for error (already performed with forwardError below).
There was a problem hiding this comment.
See my response to #30.
I think we should probably only emit events that are logical given that we are making redirection transparent. Also, we would ideally provide a similar server side experience that browserify-http, browserify-http-2 provide in the browser. I'm guessing many of the events simply aren't detectable in the browser. So I would definitely say just skip it.
There was a problem hiding this comment.
I will update the pull request to only provide response and error.
There was a problem hiding this comment.
I will update the pull request to only provide response and error.
👍 Thanks.
Are you willing to dig in on abort handling as well?
There was a problem hiding this comment.
I could have a look at abort, but that probably belongs to a different issue (where we'd discuss desired behavior etc.).
This makes the response event and callback behave as in the Node.js API. Fixes #30.
| @@ -125,8 +128,6 @@ module.exports = function(_nativeProtocols) { | |||
| }, options); | |||
| } | |||
| assert.equal(options.protocol, wrappedProtocol, 'protocol mismatch'); | |||
| options.protocol = wrappedProtocol; | |||
There was a problem hiding this comment.
No need for this assignment, as they are equal anyway.
|
@jamestalmage Anything else I can do to complete this pull request? |
|
Sorry for the delay. published as |
In #30, I remarked that the
responseevent—and hence, the passed callback—do not behave as in the original Node.js API.This pull request fixes that, by returning a proxied request rather than the original request. They proxy and the request are identical, except for event handling: the proxy only fires the
responseevent when the final redirected response arrives, whereas the (now internal)responseevent of the request fires as soon as the first (non-redirected) response arrives.