http2: The http2 server does not quickly close the stream after a frame error if a 'data' event handler is set

[murgatroid99]

• Version: 14.10.0, 12.18.3, 10.22.0
• Platform: Linux DESKTOP-OKC3QBQ 4.4.0-18362-Microsoft --abort-on-uncaught-exception prevents domains from working at all (the process crashes) #836-Microsoft Mon May 05 16:04:00 PST 2020 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: http2

What steps will reproduce the bug?

const http2 = require('http2');

const server = http2.createServer();

const trailerValue = 'X'.repeat(64 * 1024);

server.on('stream', (stream, headers) => {
  stream.respond(undefined, {waitForTrailers: true});

  // This line is necessary to trigger the bug
  stream.on('data', () => {});

  stream.on('wantTrailers', () => {
    console.log(`${new Date().toISOString()} Server sending trailers`);
    // Trigger a frame error by sending a trailer that is too large
    stream.sendTrailers({'test-trailer': trailerValue});
  });

  stream.on('frameError', (frameType, errorCode) => {
    console.log(`${new Date().toISOString()} Server frameError frameType: ${frameType}, errorCode: ${errorCode}`);
  });

  stream.on('close', () => {
    console.log(`${new Date().toISOString()} Server stream close event`);
  });

  stream.end();
});

server.listen(3000, () => {
  const clientSession = http2.connect('http://localhost:3000');

  clientSession.on('frameError', (type, errorCode, streamId) => {
    console.log(`${new Date().toISOString()} Client session frameError frameType: ${type}, errorCode: ${errorCode}, streamId: ${streamId}`);
  });

  clientSession.on('close', () => {
    console.log(`${new Date().toISOString()} Client session close event`);
    server.close();
  });

  const clientStream = clientSession.request();
  console.log(`${new Date().toISOString()} Starting client stream`);

  clientStream.on('close', () => {
    console.log(`${new Date().toISOString()} Client stream close event`);
  });

  clientStream.on('frameError', (type, errorCode, streamId) => {
    console.log(`${new Date().toISOString()} Client stream frameError frameType: ${type}, errorCode: ${errorCode}, streamId: ${streamId}`);
  });

  // This should cause the server to finish reading
  clientStream.end();
})

How often does it reproduce? Is there a required condition?

This reproduction is 100% consistent.

What is the expected behavior?

From the Http2Stream#frameError event documentation:

The 'frameError' event is emitted when an error occurs while attempting to send a frame. When invoked, the handler function will receive an integer argument identifying the frame type, and an integer argument identifying the error code. The Http2Stream instance will be destroyed immediately after the 'frameError' event is emitted.

Also, from the http2.createServer options documentation:

maxSendHeaderBlockLength <number> Sets the maximum allowed size for a serialized, compressed block of headers. Attempts to send headers that exceed this limit will result in a 'frameError' event being emitted and the stream being closed and destroyed.

Both the client and the server should see their corresponding stream object's close event soon after the 'frameError' event is emitted

What do you see instead?

On Node 10 and 12, there is a 2 minute delay between the frameError event and the close events. On Node 14, the close events are never emitted.

Node 10 and 12

2020-09-09T20:46:02.215Z Starting client stream
2020-09-09T20:46:02.227Z Server sending trailers
2020-09-09T20:46:02.228Z Server frameError frameType: 1, errorCode: -522
<2 minute delay>
2020-09-09T20:48:02.231Z Server stream close event
2020-09-09T20:48:02.233Z Client session close event
2020-09-09T20:48:02.234Z Client stream close event

Node 14

2020-09-09T20:37:45.101Z Starting client stream
2020-09-09T20:37:45.110Z Server sending trailers
2020-09-09T20:37:45.111Z Server frameError frameType: 1, errorCode: -522
<No further log lines after at least 10 minutes>

Additional information

If the data event handler is not set on the server's stream, the close event is emitted immediately on all tested Node versions.

[murgatroid99]

@nodejs/http2

[bcoe]

CC: @nodejs/http2

[RafaelGSS]

It should be fixed now (#42147). @mcollina Can you close it?