Receiving a RST_STREAM on a pushed stream causes a crash

[nwgh]

/Users/hurley/src/node-http2/lib/protocol/stream.js:627
      throw new Error('Sending illegal frame (' + frame.type + ') in ' + this.
            ^
Error: Sending illegal frame (DATA) in CLOSED state.
    at Stream.transition [as _transition] (/Users/hurley/src/node-http2/lib/protocol/stream.js:627:13)
    at Stream._pushUpstream (/Users/hurley/src/node-http2/lib/protocol/stream.js:230:8)
    at Stream._finishing (/Users/hurley/src/node-http2/lib/protocol/stream.js:351:10)
    at Stream.emit (events.js:104:17)
    at finishMaybe (_stream_writable.js:484:14)
    at endWritable (_stream_writable.js:493:3)
    at Stream.Writable.end (_stream_writable.js:459:5)
    at OutgoingResponse._finish (/Users/hurley/src/node-http2/lib/http.js:348:17)
    at OutgoingResponse.emit (events.js:129:20)
    at finishMaybe (_stream_writable.js:484:14)

This happens when Firefox refuses a pushed stream (using RST_STREAM), either because of configuration (disabling network.http.spdy.allow-push) or because of a bug in firefox (https://bugzilla.mozilla.org/show_bug.cgi?id=1127618). Presumably, this happens with any other UA that refuses pushes in a similar fashion.

My first suspicion is that there's a race between receipt of the RST_STREAM (which sets the stream state to CLOSED) and stopping sending of data on the pushed stream that causes us to hit this state, but I have to investigate further to be sure.

[bnolan]

I'm getting this bug as well, when I'm requesting several dozen files via XHR in chome, and then stop the page loading.

[lewispham]

I got a similar error while I was trying to push multiple files with this code.

var server = http2.createServer(options,function(req,res){
    fileList.forEach(function(path){
        var push = res.push(path);
        fs.createReadStream(path).pipe(push);
        push.writeHead(200);
    });
    fs.createReadStream(req.url).pipe(res);
    res.writeHead(200);
});
server.listen(8080);

[adamhenson]

I'm reproducing this issue consistently with browser cached resources, and intermittently when the resources aren't cached. So, I agree that this is probably a race condition. My issue occurs with one file or multiple files. I've only tried on Chrome. It seems that the client is trying to cancel the pushed stream by sending a RST_STREAM frame to the server. I just don't really know how to handle that. Is there documentation somewhere? Seems like I should be able to receive that and send back a 304 status somehow. Regardless, it is crashing the server - so, I'll look forward to any conclusion.

This is what I see in the log:

18:06:44.280Z ERROR server/stream: Sending illegal frame. (e=1, s=5, state=CLOSED, closedByUs=false, closedWithRst=true)
    frame: {
      "id": 0,
      "type": "DATA",
      "flags": [
        "END_STREAM"
      ],
      "stream": 6,
      "data": "",
      "length": 0
    }

And this is my code:

function onRequest(request, response) {
  if(response.push) {
    FILES.forEach((file, index) => {
      let push = response.push(file.path);
      push.writeHead(200, file.headers);
      fs.createReadStream(path.join(__dirname, file.path)).pipe(push);
      if(index === FILES.length - 1) response.end(Template.output(FILES));
    });
  }
}

[akc42]

I have found a workaround for this which is showing some interesting results. In particular, I am failing on one specific file, which is supposed to being loaded as a background image in css, and my app itself is not showing this image.

Anyway, the workaround

    pushdata(res,pushlist) {

      pushlist.forEach(line => {
        const push = res.push(line);
        const l = line;
        push.stream.on('error', err => {
          debug('Error pushing %s message %s', l, err.message);
          push.stream.removeAllListeners();
        });
        push.stream.on('end', () => {
          push.stream.removeAllListeners();
        })
        push.writeHead(200);
        debug('Pushing file %s',APP_ROOT+line);
        fs.createReadStream(APP_ROOT+line).pipe(push);
      });

    }

By putting on 'error' on the push.stream variable it catches the errors and allows me to continue. I am now regularly pushing 14 files when I see a request for '/' or '/index.html' and its normally one of the early files (the background image) that is failing.

[akc42]

I have been conducting some tests with different browsers to see where things are. I have altered my application so I can switch push on and off with a simple environment variable change. When push is turned on I push about 40 files when I first load the application (index.html), and then another 10 or so when the user cookie has been checked and the application requests the element (a custom web component) that controls all the pages of the application.

With push turned on:

Linux Chrome: Works almost perfectly, occasional failures whilst pushing (using the on error callback as shown above). I also sometimes, but the majority of the time not, then experience the crash
Error: Sending illegal frame (DATA) in CLOSED state.
Linux Firefox: Same as Chrome
Android Chrome: Seems to work perfectly. Not run long enough to see if occasional failures
Windows Chrome: Works almost perfectly. Not run long enough to see if occasional failures
Windows Firefox: starts to display a page, but then gets "Stuck" - I think after the first massive push
Windows Edge starts to display a page, but then gets Stuck
Macbook Air: Chrome works As per Linux Chrome
Macbook Safari: Doesn't work. Hangs immediately it request the page. I don't even see the request in the server (by that I mean the callback in http2.createServer that gets called for every request doesn't get called)
IPad: Safari doesn't work. No response.
Macbook Firefox; Works As per Linux Chrome

With Push turned on and @adamhenson fix #107 applied

The two Windows browsers (Firefox and Edge) that got stuck now no longer get stuck
Macbook Safari and IPad still don't work

With Push Notifications turned off

All browsers in all environments work.

Given a primary aim is to support IPads I will have to leave push turned off for now (at least until Apple's performance improve), but I would like to know what to do to detect the inability of a browser to handle push, both in general - and when I have pumped too much at them.

[nirsharma]

I am not even pushing anything. just the regular request response and i get this.

Me too, I am not pushing anything, just fastly making several regular requests:

$('#image').attr('src','http://192.168.1.102:8080/image.jpg?version='+i);

I got this:

Error: Sending illegal frame (DATA) in CLOSED state.
    at Stream.transition [as _transition] (/home/jondoe/node_modules/http2/lib/protocol/stream.js:628:33)
    at Stream._pushUpstream (/home/jondoe/node_modules/http2/lib/protocol/stream.js:230:8)
    at Stream._finishing (/home/jondoe/node_modules/http2/lib/protocol/stream.js:351:10)
    at emitNone (events.js:86:13)
    at Stream.emit (events.js:185:7)
    at finishMaybe (_stream_writable.js:488:14)
    at endWritable (_stream_writable.js:498:3)
    at Stream.Writable.end (_stream_writable.js:463:5)
    at OutgoingResponse._finish (/home/jondoe/node_modules/http2/lib/http.js:352:17)
    at emitNone (events.js:86:13)
    at OutgoingResponse.emit (events.js:185:7)

[nirsharma]

@lmbao #210
with this PR, it's working.