saveRequestFiles doesnt delete file when user cancels request

[SupertigerDev]

Prerequisites

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

Fastify version

5.0.0

Plugin version

9.0.1

Node.js version

21

Operating system

Linux

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

Ubuntu 20.04

Description

1. in postman, attach a large file
2. Post the request and then cancel it
3. See that the file is not deleted

Link to code that reproduces the bug

import Fastify from "fastify";
import multipart from "@fastify/multipart";
const fastify = Fastify({
  logger: true,
});
fastify.register(multipart, { logLevel: "error" });

fastify.post("/", async (request, reply) => {
  const files = await request.saveRequestFiles({
    limits: { files: 1, fields: 0, fileSize: 55655 * 1024 * 1024 },
  });

  reply.send({ test: "lol" });
});

fastify.listen({ port: 3000 });

edit: infact, it seems like saveRequestFiles promise just gets stuck forever or something

Expected Behavior

file should be deleted

Seems like the solution would be to add a catch block to pipeline and unlink the file

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[IqbalLx]

@mcollina can this issue solved by this https://fastify.dev/docs/latest/Guides/Detecting-When-Clients-Abort/ ? when client abort, delete the file?? any other info I should know? if not I'll send PR for this

[mcollina]

Go ahead and do it! Yes, that should do.

[IqbalLx]

I successfully created a test for this scenario, but it turns out the savedRequestFiles key has a null value when accessed from the onRequest hooks. I reviewed the Fastify lifecycle documentation, and I assumed that since saveRequestFiles() is called inside the User Handler, it should modify the request object in the parent lifecycle as well, causing savedRequestFiles to have a value. Am I missing something here?

fastify.addHook('onRequest', async (request) => {
    request.raw.on('close', async () => {
      if (request.raw.aborted) {
        // both of this key has null values
        console.log(request.tmpUploads)
        console.log(request.savedRequestFiles)
      }
    })
  })

both values is being set here, inside saveRequestFiles() function that supposed to be called from User Handler

async function saveRequestFiles (options) {
    // .... truncated
    for await (const file of files) {
      const filepath = path.join(tmpdir, generateId() + path.extname(file.filename))
      const target = createWriteStream(filepath)
      try {
        await pump(file.file, target)
        this.savedRequestFiles.push({ ...file, filepath })
        this.tmpUploads.push(filepath)
      } catch (err) {
        this.log.error({ err }, 'save request file')
        throw err
      }
    }

    return this.savedRequestFiles
  }

[mcollina]

Can you send a PR with your failing test/partial solution? I'll take a look

[IqbalLx]

Sure, here it is #567

[mcollina]

I can't really reproduce the problem. @SupertigerDev can you provide a script to reproduce it?

[SupertigerDev]

idk how to reproduce it with script, try my steps, upload a large file. maybe with script, upload a file and reload the page in the middle of uploading. It's also been a while since used this. I moved to an alternative, sorry

[mcollina]

I successfully created a test for this scenario, but it turns out the savedRequestFiles key has a null value when accessed from the onRequest hooks. I reviewed the Fastify lifecycle documentation, and I assumed that since saveRequestFiles() is called inside the User Handler, it should modify the request object in the parent lifecycle as well, causing savedRequestFiles to have a value. Am I missing something here?

That's normal related to the test: onRequest is executed before the the saveRequestFiles call, so the two arrays are empty.

[mcollina]

I've got it. I've updated #567.