fastify / fastify-reply-from

fastify plugin to forward the current http request to another server
MIT License
151 stars 76 forks source link

Undici Body Timeout Error crashes fastify server #328

Closed paymog closed 12 months ago

paymog commented 1 year ago

Prerequisites

Fastify version

4.1.0

Plugin version

9.0.1

Node.js version

18

Operating system

Linux

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

alpine

Description

I often get full process crashes with stack traces like the following

node:events:495
      throw er; // Unhandled 'error' event
      ^

BodyTimeoutError: Body Timeout Error
    at Timeout.onParserTimeout [as callback] (/app/node_modules/undici/lib/client.js:930:28)
    at Timeout.onTimeout [as _onTimeout] (/app/node_modules/undici/lib/timers.js:20:13)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7)
Emitted 'error' event on BodyReadable instance at:
    at BodyReadable.emit (/app/node_modules/undici/lib/api/readable.js:66:18)
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'UND_ERR_BODY_TIMEOUT'
}

The only place where I use undici is through this plugin. I was using the following config

        undici: {
          bodyTimeout: 15000,
          headersTimeout: 15000,
          connections: 50,
        },

and when I bumped the body timeout to 30000 the issue went away. However, I'd like to be able to figure out why my whole server crashes when one of these timeouts is encountered.

I'm using this plugin like so:

// routes.ts
fastify.post('path', async (req, reply) => {
  // stuff...
  // last call in the path
  await loadBalancer.replyFromBackend(request, reply);
})

// loadbalancer.ts
  public async replyFromBackend(request: FastifyRequest, reply: FastifyReply) {
     // lots of stuff
     const { buffer: currentResponseBuffer, rep } = await this.replyFrom(request, reply, chainNode);

     // manipulate rep and currentResponseBuffer into a variable called finalResponseBuffer
    reply.send(finalResponseBuffer)
  }

// also in loadBalancer.ts 
  private replyFrom(_: FastifyRequest, reply: FastifyReply, chainNode: EnhancedNode) {
    return new Promise<{ buffer: Buffer; rep: FastifyReply }>((resolve, reject) => {
      const pathWithQuery = chainNode.pathname ? `${chainNode.pathname}${chainNode.queryString}` : '/';

      // this does not actually return a promise
      // eslint-disable-next-line @typescript-eslint/no-floating-promises
      reply.from(pathWithQuery, {
        getUpstream: () => chainNode.origin,
        onError: (_rep, err) => {
          reject(`Error: ${err.error.message}`);
        },
        onResponse(_, rep, res) {
          const chunks: Uint8Array[] = [];

          res.on('data', (chunk: Uint8Array) => {
            chunks.push(chunk);
          });

          res.on('end', () => {
            const bodyBuffer = Buffer.concat(chunks);
            resolve({ buffer: bodyBuffer, rep: rep as FastifyReply });
          });
        },
      });
    });
  }

Steps to Reproduce

Unfortunately I don't know how to reproduce since it happens quite rarely (1 request per 10million or something like that)

Expected Behavior

I expect body parsing errors to not kill the entire server but just break the request on which the error happens.

mcollina commented 1 year ago

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.

paymog commented 1 year ago

Unfortunately I don't know how to reproduce since it happens quite rarely (1 request per 10million or something like that).

mcollina commented 1 year ago

My guess is that you are missing an error handler inside onResponse, e.g.:

        onResponse(_, rep, res) {
          const chunks: Uint8Array[] = [];

          res.on('data', (chunk: Uint8Array) => {
            chunks.push(chunk);
          });

          res.on('error', (err: Error) => {
            reject(err);
          });

          res.on('end', () => {
            const bodyBuffer = Buffer.concat(chunks);
            resolve({ buffer: bodyBuffer, rep: rep as FastifyReply });
          });
        },
Uzlopak commented 1 year ago

If increasing the timeout decreases the chance to reproduce it, then decreasing the timeout to the minimum should increase te chance to reproduce it.

paymog commented 12 months ago

@mcollina you have incredible intuition. I just deployed that suggestion and it seems to have resolved the issue. Thank you!