Handle socket errors in server code

[snorp]

Right now, in server code, nothing really happens if the socket goes away. All streams remain open and attempting to send data, outgoing frames are queued, etc. It is not totally clear to me from reading the spec what should happen. A few thoughts:

• We could respond to the socket with a GOAWAY frame. Probably won't get there, but seems "right".
• We need some way to close the currently open streams from the server's point of view. I think just setting their state to CLOSED is the right thing (and emitting appropriate events in the OutgoingResponse object), but the spec doesn't really say if that's valid or not.
• Another option would be to not modify any of the actual HTTP state in the stream, but simply emit a 'close' or 'error' event on the OutgoingResponse object. That way we aren't faking any state changes, but applications are still able to stop sending data to a dead client.

[nwgh]

I think a combination of (1) and (3) in your list is probably the best bet here at first glance, but I have to think about it more to be totally sure.

[snorp]

I thought about it some more this weekend.

I think the spec actually says we are required to do 1) according to 5.4.1

There is a small mention in 5.4.3 about what to do regarding open streams if the socket goes down. The wording seems to leave it up to the implementation on what to do. I'm leaning towards synthesizing a received RST_STREAM frame with a CANCEL or PROTOCOL_ERROR.

[molnarg]

How about implementing this in the http2-protocol module:

• if the Connection object receives an end of stream signal (basically a connection.write(null)) and this._closed === false (there was no normal connection termination)
  • take the same code path as if we received a GOAWAY with PROTOCOL_ERROR code
  • this code path should write an end of stream signal (null) into the individual streams (on the stream's upstream side) so that they can decide 'themselves' how to deal with it
  • and emit an error on the connecton object (already implemented)
• If an individual stream receives an end of stream signal and this._closed === false
  • take the same code path as if it received an RST_STREAM with PROTOCOL_ERROR code
  • this code path emits an error on the stream (already implemented)

I believe that node-http2 module already forwards error events from http2-protocol Stream and Connection, so the only thing that has to be done is writing an end of stream signal into the connection object in case a TCP error occures. When the TCP connection is normally terminated, the signal is written to the Connection object automatically since they are piped together (AFAIK the pipe does not propagate errors automatically so that part must be done manually).

[snorp]

I wrote something last night that is pretty close to what you describe here. I just put it in a PR here: https://github.com/molnarg/node-http2-protocol/pull/25

There is a companion node-http2 PR here: https://github.com/molnarg/node-http2/pull/75 (I messed up and didn't attach it to this issue)

[molnarg]

Thanks, I'll try to review it ASAP, probably on the weekend!

[nwgh]

@snorp do you know offhand if this is still a thing? (hell, do you even care?) trying to triage and do some bug cleanup around these parts

[snorp]

It's probably still a thing, but no, I don't care :)