Hard crash under Node 24 when handling aborted requests

[gwicks]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

5.4.0

Plugin version

12.2.0

Node.js version

24.4.1

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Alpine 3.21 (ARM64)

Description

After upgrading to Node 24.x, we started having erratic hard crashes in production, seemingly when connections to the upstream server aren't closing quite right.

 Node.js v24.4.1 
   at process.processTicksAndRejections (node:internal/process/task_queues:90:21) 
  at emitErrorCloseNT (node:internal/streams/destroy:129:3) 
   at emitErrorNT (node:internal/streams/destroy:170:8) 
Emitted 'error' event on BodyReadable instance at: 
 at wasm://wasm/0002f80e:wasm-function[20]:0x67b2 
  at wasm://wasm/0002f80e:wasm-function[10]:0x474 
at wasm_on_headers_complete (/usr/src/app/node_modules/undici/lib/dispatcher/client-h1.js:108:30) 
 at Parser.onHeadersComplete (/usr/src/app/node_modules/undici/lib/dispatcher/client-h1.js:514:27) 
at Request.onHeaders (/usr/src/app/node_modules/undici/lib/core/request.js:244:29) at RequestHandler.onHeaders (/usr/src/app/node_modules/undici/lib/api/api-request.js:140:14) 
at RequestHandler.runInAsyncScope (node:async_hooks:214:14) 
  at /usr/src/app/node_modules/@fastify/reply-from/lib/request.js:187:7 
 at /usr/src/app/node_modules/@fastify/reply-from/index.js:299:9 
 at /usr/src/app/node_modules/@fastify/reply-from/index.js:215:20 
TypeError: res.stream.close is not a function 
  throw er; // Unhandled 'error' event 
node:events:485 

Looking deeper at the code, it looks like Node 24 changes the structure of the stream API and removes the close method. Additionally, we are using the HTTP1 backend. The upstream server in my case is a kind of rickety Ruby on Rails legacy server, and it prone to periodically dropping requests/closing unexpectedly under certain conditions. Previously, this was handled just fine.

Here's some of my config for @fastify/http-proxy for reference that's feeding into reply-from, can't include all of it as it's somewhat sensitive.

      onResponse: async (
        request,
        reply,
        res: RawReplyDefaultExpression<RawServerBase>
      ) => {
        if (request.currentSubspan) {
          request.currentSubspan.setAttribute(
            "reply.elapsedTime",
            reply.elapsedTime
          );
          if (reply.statusCode >= 400) {
            request.totalRailsErrorRespsonseCounter.add(1, {
              status_code: reply.statusCode,
              error_type: "upstream_error",
            });
            request.currentSubspan
              .setStatus({ code: SpanStatusCode.ERROR })
              .end();
          } else {
            request.currentSubspan.setStatus({ code: SpanStatusCode.OK }).end();
          }
          request.totalRailsRequestCounter.add(1);
          request.currentSubspan = null;
        }
        if ((res as unknown as UndiciProxyResponse)?.stream) {
          reply.send((res as unknown as UndiciProxyResponse)?.stream);
        } else {
          reply.send(res);
        }
      },

Link to code that reproduces the bug

N/A, relies on requests being aborted under HTTP/1 and Node 24.x

Expected Behavior

It should gracefully handle an aborted connection without throwing an unhandled exception.

One possible fix for the offending code might be something like

  if (sourceHttp2 && typeof res.stream.close === 'function') {
    // HTTP/2 stream
    res.stream.close(NGHTTP2_CANCEL)
  } else if (typeof res.stream.destroy === 'function') {
    // HTTP/1.1 stream or other stream types
    res.stream.destroy()
  }

Inside requestImpl.

[gunters63]

@gwicks : Thanks for the thorough analysis. Yes, it seems the close call is deprecated and was removed in Node 24. destroy() does replace it. But with destroy() it is not possible to send a RST code. Maybe we need two calls here.

I will extend the Test I added in my commit with a HTTP/1 path and run them with Node 24 to be sure it worls. I think I can make a PR with a fix friday.

[gwicks]

Looking at the docs for Node, it seems like .close just doesn't exist on HTTP/1 connections and hasn't for a while. I specifically don't use HTTP/2 since there's no benefit (the upstream is HTTP/1 only).

https://nodejs.org/docs/latest-v22.x/api/stream.html
https://nodejs.org/api/stream.html

Seems that this would've also failed under Node 22.x as well, but it's not super clear. My previous build that was working didn't have your code at all and was running 12.1.0. I will say I have noticed the janky reset/closing behaviour with streams previously, so I understand what you were trying to fix.

[gwicks]

I did wire up my debugger as well and yeah, on both paths, HTTP/2 and HTTP/1, close is not a valid function within Readable it seems.

[gunters63]

I am currently abroad so I am not 100% sure, but my dev computer at home should use Node 24, so that's strange.

At least for HTTP/2 and Node 20/22 stream.close is still there, see the test results for the PR: #426. Maybe the CI tests should also include Node 24.

When I get home tomorrow I will also check the source of Node 22 and 24 to see what is the equivalent of close(RST code) and make a PR which fixes the problem.

[gunters63]

Actually the CI tests are also run with Node 24, not sure what's going on here.
Anyway, I will also add a HTTP/1 path to the relevant test.

[gwicks]

I think it may just be an HTTP/2 vs. HTTP/1 thing. I'll take a look at your PR if you open one to fix it and test it with my use-case when I'm back home next week.

[mcollina]

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

[gwicks]

Wish I could make it in code-form, but the best steps to reproduce would be to make a request with reply-from to a slow-responding server, and interrupt/abort the request. Make sure you're running Node 24.x and you're using HTTP/1. When it falls into the handler code introduced in 12.2.0, it'll throw an exception and crash the Node process. I can try and make something small that uses HTTPBin's delay endpoint but it's been down lol.

[gunters63]

I wrote a little additional test like that:

t.test('http2 -> http canceled streams cleanup', async (t) => {
  const instance = Fastify({
    http2: true,
    https: httpsOptions
  })

  t.after(() => instance.close())

  const target = http.createServer()

  target.on('request', (_, response) => {
    const sendData = () => {
      response.writeHead(200, { 'Content-Type': 'text/plain' })
      response.end('ok')
    }

    // delay sending data
    setTimeout(sendData, 50)
  })

  instance.get('/', (_request, reply) => {
    reply.from()
  })

  t.after(() => target.close())

  target.listen()
  await once(target, 'listening')

  instance.register(From, {
    base: `http://localhost:${target.address().port}`,
  })

  const url = await instance.listen({ port: 0 })

  const client = http2.connect(url, {
    rejectUnauthorized: false,
  })

  // see https://github.com/fastify/fastify-reply-from/issues/424
  // without the bug fix this will fail after about 15 requests
  for (let i = 0; i < 30; i++) { await makeRequest(client, i) }

  client.close()
  instance.close()
  target.close()
})

This will indeed hit the line with res.stream.close and close is indeed undefined for a HTTP/1 connection:

      if (this.request.raw.aborted && res.stream) {
        // the request could have been canceled before we got a response from the target
        // forward this to the upstream server and close the stream to prevent leaks
        res.stream.close(NGHTTP2_CANCEL) <--- close is undefined!
        // no need to send a reply for aborted requests or call the onResponse callback
        return
      }

although the internal error handling of Fastify swallows the exception. I didn´t experience a hard crash (tested with Node 24). It should be possible to expose the error though and create a failing test.

The simplest fix would be likely indeed just to test for stream.close to exist, like that:

      if (this.request.raw.aborted && res.stream && res.stream.close === 'function') {
        // the request could have been canceled before we got a response from the target
        // forward this to the upstream server and close the stream to prevent leaks
        // this cleanup is only needed for http2, where the stream was explicitly created by reply-from and has a close method
        res.stream.close(NGHTTP2_CANCEL)
        // no need to send a reply for aborted requests or call the onResponse callback
        return
      }

I will try to make the http2 -> http test fail without the fix and create a PR this weekend.