metcoder95 / fastify-racing

Cancel any running operation at the right time on your request handler
MIT License
12 stars 1 forks source link

Requests with multipart/form-data flagged as aborted immediately after payload consumed #62

Open simosho opened 1 week ago

simosho commented 1 week ago

Describe the bug When making post/put/patch requests to endpoints that accept multipart/form-data uploads (e.g., using@fastify/multipart), fastify-racing incorrectly triggers the abort signal for every request after the payload has been parsed.

To Reproduce Run the following fastify app

// app.js
const fastify = require('fastify')()
fastify.register(require('fastify-racing'))
fastify.register(require('@fastify/multipart'))

fastify.post('/data', async (request, reply) => {
  const signal = request.race()
  const parts = request.parts()
  const payload = {}

  console.log('Signal before', signal.aborted) // false
  for await (const part of parts) {
    if (part.file) {
      payload[part.fieldname] = await part.toBuffer() // to consume the stream
    }
  }
  console.log('Signal after', signal.aborted) // true
  await fetch('https://example.com', { signal }) // as a result, calls like this will always be aborted

  return "Ok"
})

const start = async () => {
  await fastify.listen({ port: 3000 })
  console.log('Server running on http://localhost:3000')
}
start()

Then call the endpoint, e.g. using curl (assumes app.js is in the same directory):

curl -X POST http://localhost:3000/data -F "file=@app.js"

results in

{"statusCode":500,"code":"20","error":"Internal Server Error","message":"This operation was aborted"}

Expected behavior Expect the signal to not be aborted yet.

Desktop (please complete the following information):

Additional context It is probably related to https://github.com/fastify/help/issues/866. The request.raw.closed event is fired as soon as the request payload is consumed, so unfortunately, it is not a reliable way to detect if the request was aborted. There is probably no easy fix, if there is one at all.

metcoder95 commented 3 days ago

Yeah, is pretty much what is described in the issue you referenced.

Nevertheless the bug can be somehow addressed; here we need to check wether the connection is dropped at the moment of the request being in flight.

If the close has been emitted, and the request body is completed/consumed; as long as the connection has not been dropped, the signal can not be aborted yet (in the case of H2, as long as the stream stills alive, the signal should not be aborted).

simosho commented 2 days ago

So it should rather listen to the raw.socket.once('close') event then, or what do you propose? I can create a PR if you want

metcoder95 commented 1 day ago

Yeah, exactly. close is only emitted if the connection has been dropped. There we just need to ensure that the request hasn't been served yet (as it will remain untouched if the connection is a keep alive and no request is coming).

A PR is really welcomed