search

spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

response: fix missing socket error #155

Closed jonathanong closed 10 years ago

jonathanong commented 10 years ago

not sure why this is, but sometimes this.socket is empty. seems like it sometimes exists on req but not res.

currently, the code is borked because you have a stream variable inside .push() as well as outside. now, the error is properly emitted.

also moved the error outside the next tick so there's a proper stack trace

jonathanong commented 10 years ago

okay it seems like somewhere res.socket = null is being set after the response is finished. is there a reason for that?

indutny commented 10 years ago

I think yes, there is a reason for it. That's a behavior of node.js and it is unlikely to change.

Could you please remove unrelevant changes in your patch?

indutny commented 10 years ago

And add test, perhaps?

jonathanong commented 10 years ago

which parts are unrelevant? the stream stuff is a scoping error. putting the new Error in the next tick unnecessarily destroys the stack trace

indutny commented 10 years ago

Convinced, landed in de057d1 :) Thank you!