HTTP/2 GOAWAY event crashes application (Undici v6.20.1)

[bdentino]

Bug Description

In release 6.20.1, there appears to be a regression in http/2 support which crashes Node with a TypeError on handling HTTP/2 GoAway events. It appears to be an error introduced by this change (specifically here), which adds a call to util.errorRequest(client, request, err) when request is undefined. To be honest, I'm not sure if the root of the problem is that request is undefined and it shouldn't be, or if that's an allowed scenario which just needs an undefined check.

The problem does not appear in 6.20.0, and I added a simple check for undefined before calling util.errorRequest(client, request, err) in 6.20.1 which prevents the app from crashing but I'm not familiar enough with the internals here to know if this is an acceptable solution or a deeper investigation is required to understand why request is undefined.

Reproducible By

This is reproducible in my project/environment by simply waiting 6-7 minutes after issuing some http/2 requests. A 'goaway' event is eventually received and the app crashes. Unfortunately this is a large project in a private repo which I can't share. If it's not obvious to the maintainers what the problem is based on the description/screenshots here, I will be happy to try and create a simple shareable repro.

Expected Behavior

Receiving a goaway event on an http/2 session should not crash the application.

Logs & Screenshots

Environment

Node v22.10.0, Docker node:22-alpine image

Additional context

[ronag]

@metcoder95

[metcoder95]

I imagine this can be reproducible with a simple http2 server that replies with goaway frame.

If you can find a way to provide an
Minimum Reproducible Example that isolates the issue, that will be pretty much appreciated; I'll take a look later this week

[bdentino]

@metcoder95 thanks for taking a look - I will make some time this weekend to set up a reproduction

[bdentino]

Okay here's a simple repro script:

const { readFileSync } = require('fs');
const http2 = require('http2');
const { Client } = require('undici');

// generate cert & key as pem files
// openssl req -newkey rsa:2048 -new -nodes -x509 -days 3650 -keyout key.pem -out cert.pem

const cert = readFileSync('./cert.pem');
const key = readFileSync('./key.pem');

// Create an HTTP/2 server
const server = http2.createSecureServer({ cert, key, allowHTTP1: true }, (req, res) => {
  console.log('got request')
  const { socket: { alpnProtocol } } = req.httpVersion === '2.0' ? req.stream.session : req;
  res.writeHead(200, { 'content-type': 'application/json' });
  res.end(JSON.stringify({
    alpnProtocol,
    httpVersion: req.httpVersion,
  }));
});

// Trigger goaway after 10 seconds
server.on('session', (session) => {
  console.log('Session established');
  setTimeout(() => {
    console.log('Sending goaway');
    try { session.goaway(); } catch (e) { console.error('failed to send goaway') }
  }, 10000)
})

// Ignore self-signed cert errors
process.env["NODE_TLS_REJECT_UNAUTHORIZED"] = 0;

server.listen(8080, () => {
  console.log('Server listening on port 8080');
  const client = new Client('https://localhost:8080', { allowH2: true })
  client.request({ method: 'GET', path: '/' }, (err, { statusCode, body }) => {
    console.log('Waiting for GOAWAY event (should crash in about 10 seconds)...');
    if (err) return console.error(err);
    console.log(`Response received with status code: ${statusCode}`);
    body.text().then(console.log);
  });
});

// Handle uncaught exceptions
process.on('uncaughtException', (err) => {
  console.error('Uncaught Exception:', err);
  process.exit(1);
});

[metcoder95]

Thanks! I'll add it as tests case; already have WIP