Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
simosho opened this issue Nov 14, 2024 · 3 comments · May be fixed by #63
Open

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

simosho opened this issue Nov 14, 2024 · 3 comments · May be fixed by #63
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@simosho
Copy link

simosho commented Nov 14, 2024

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 "[email protected]"

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 fastify/help#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
Copy link
Owner

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).

@metcoder95 metcoder95 added bug Something isn't working good first issue Good for newcomers labels Nov 19, 2024
@simosho
Copy link
Author

simosho commented Nov 20, 2024

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

@simosho simosho closed this as completed Nov 20, 2024
@simosho simosho reopened this Nov 20, 2024
@metcoder95
Copy link
Owner

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

@simosho simosho linked a pull request Nov 22, 2024 that will close this issue
@metcoder95 metcoder95 linked a pull request Nov 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants