molnarg / node-http2

An HTTP/2 client and server implementation for node.js
MIT License
1.79k stars 185 forks source link

Server may crash when a request's socket is ended manually #196

Open JiaJiaJiang opened 8 years ago

JiaJiaJiang commented 8 years ago

I use req.socket.end() to drop the requests that I won't response anything at all. This performances well with https module,but will crash with http2.

here is a crash log I got:

....../node_modules/http2/lib/protocol/connection.js:302
        var frame = stream.upstream.read((this._window > 0) ? this._window : -1);
                          ^

TypeError: Cannot read property 'upstream' of undefined
    at Connection._send (....../node_modules/http2/lib/protocol/connection.js:302:27)
    at processImmediate [as _immediateCallback] (timers.js:383:17)
nwgh commented 8 years ago

First off, yes, this is a bug. However - you shouldn't be manually closing down sockets with HTTP/2. If you wish to not respond to a request, you should instead send a RST_STREAM with error code REFUSED_STREAM. (Or, if you're feeling exceedingly mean to the peer, you can just ignore it - HTTP/2 is an inherently parallel protocol.) There is no reason to close down the socket unless something has gone horribly wrong on the connection (pretty much all of these are enumerated by 7540, and are handled internally by node-http2).

JiaJiaJiang commented 8 years ago

Thank you for your suggestion. By the way,"you can just ignore it" means I can just not do anything on the request?Will these request objects keep in node?

nwgh commented 8 years ago

The request object will hang around, so just ignoring things isn't exactly suggested (unless you're ignoring them until a certain point in the future), but it won't break communication immediately (until you run out of memory).

JiaJiaJiang commented 8 years ago

Thanks for your answer.I've understood how it works.Should I leave the issue here or close it?