nodejs / undici

An HTTP/1.1 client, written from scratch for Node.js
https://nodejs.github.io/undici
MIT License
6.19k stars 540 forks source link

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

Open bdentino opened 1 day ago

bdentino commented 1 day ago

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

Screenshot 2024-10-21 at 1 53 36 PM

Environment

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

Additional context

ronag commented 1 day ago

@metcoder95

metcoder95 commented 1 day ago

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