backport: fix: make sure the handler resolves in all cases (#515)

Author: gurgunday

index.js

@@ -232,27 +232,27 @@ function fastifyMultipart (fastify, options, done) {
     bb
       .on('field', onField)
       .on('file', onFile)
+      .on('end', cleanup)
+      .on('finish', cleanup)
       .on('close', cleanup)
-      .on('error', onEnd)
-      .on('end', onEnd)
-      .on('finish', onEnd)
+      .on('error', cleanup)
 
     bb.on('partsLimit', function () {
       const err = new PartsLimitError()
       onError(err)
-      process.nextTick(() => onEnd(err))
+      process.nextTick(() => cleanup(err))
     })
 
     bb.on('filesLimit', function () {
       const err = new FilesLimitError()
       onError(err)
-      process.nextTick(() => onEnd(err))
+      process.nextTick(() => cleanup(err))
     })
 
     bb.on('fieldsLimit', function () {
       const err = new FieldsLimitError()
       onError(err)
-      process.nextTick(() => onEnd(err))
+      process.nextTick(() => cleanup(err))
     })
 
     request.pipe(bb)
@@ -378,18 +378,15 @@ function fastifyMultipart (fastify, options, done) {
       currentFile = null
     }
 
-    function onEnd (err) {
-      cleanup()
-
-      ch(err || lastError)
-    }
-
     function cleanup (err) {
       request.unpipe(bb)
-      // in node 10 it seems that error handler is not called but request.aborted is set
+
       if ((err || request.aborted) && currentFile) {
         currentFile.destroy()
+        currentFile = null
       }
+
+      ch(err || lastError || null)
     }
 
     return parts

test/multipart.test.js

@@ -634,3 +634,58 @@ test('should not miss fields if part handler takes much time than formdata parsi
   await once(res, 'end')
   t.pass('res ended successfully')
 })
+
+test('should not freeze when error is thrown during processing', { skip: process.versions.node.startsWith('14') }, async function (t) {
+  t.plan(2)
+  const app = Fastify()
+
+  app
+    .register(multipart)
+
+  app
+    .post('/', async (request, reply) => {
+      const files = request.files()
+
+      for await (const { file } of files) {
+        try {
+          const storage = new stream.Writable({
+            write (chunk, encoding, callback) {
+            // trigger error:
+              callback(new Error('write error'))
+            }
+          })
+
+          await pump(file, storage)
+        } catch {}
+      }
+
+      return { message: 'done' }
+    })
+
+  await app.listen()
+
+  const { port } = app.server.address()
+
+  const form = new FormData()
+  const opts = {
+    hostname: '127.0.0.1',
+    port,
+    path: '/',
+    headers: form.getHeaders(),
+    method: 'POST'
+  }
+  const req = http.request(opts)
+
+  try {
+    form.append('upload', fs.createReadStream(filePath))
+    form.pipe(req)
+  } catch {}
+
+  const [res] = await once(req, 'response')
+  t.equal(res.statusCode, 200)
+  res.resume()
+  await once(res, 'end')
+  t.pass('res ended successfully!')
+
+  await app.close()
+})