Express.JS: How handle errors with Busboy?

Question

I need to catch all error into on file event of busboy and pass the error to next function.

I want also catch all busboy error and pass the error to next function.

import * as Busboy from 'busboy';

uploadAction(req: Request, res: Response, next: NextFunction) {
    let busboy: busboy.Busboy;
    // Create a new busboy instance
    // from doc, busboy constructor can raise an exception
    try {
        busboy = new Busboy({ headers: req.headers });
    } catch (err) {
        return next(err);
    } 
           
    busboy.on('file', (fieldname, file, filename) => {
        try {
            // my logic here...  
            throw new Error('Foo');  
        } catch (error) {
            // emit error to standard busboy event
            busboy.emit('error', error);
        }   
    });

    // listen all busboy error
    busboy.on('error', (err: string) => { 
        // remove busboy pipe
        req.unpipe(busboy);
        
        // ???????????????????????????????????
        // copy from https://github.com/expressjs/multer/blob/master/lib/make-middleware.js#L50
        req.on('readable', req.read.bind(req));
        busboy.removeAllListeners();

        // send error to next function
        next(err);
    });

    req.pipe(busboy);
}

the next function is a global error handler that elaborate the error and send a response, eg.:

app.use((err: Error, req: Request, res: Response, next: NextFunction) => {
    // some logic here...
    res.send({ error: error.message });
});

Now, whitout the line

req.on('readable', req.read.bind(req));

into:

busboy.on('error', (err: string) => { 
    // remove busboy pipe
    req.unpipe(busboy);
        
    // ???????????????????????????????????
    // copy from https://github.com/expressjs/multer/blob/master/lib/make-middleware.js#L45
    req.on('readable', req.read.bind(req));
    busboy.removeAllListeners();

    // send error to next function
    next(err);
});

the error handler can't send the response to the client..

But what is the purpose of:

req.on('readable', req.read.bind(req));

In general: how handle correctly busboy error?

Answer 1

The errors that can happen during a file upload are:

1. timeouts, aborts in case you get req.on('aborted'),
2. individual handlers failing, meaning calls started in your busboy.on('field'/'file'/'finish') fail either synchronously or asynchronously (or stream in 'file' emits error)
3. busboy emits 'error', for example, in case of a malformed multipart/form-data request

In every case you'd want to stop wasting resources and stop busboy from processing any more events, clean up what you've done so far and return to the caller as soon as possible instructing it to terminate the ongoing upload.

Since we don't want to repeat ourselves, in the Express.js route handler, we'd place all this logic in one common place.

You can stop busboy from processing more events by calling req.unpipe() and routing all work through a work queue that you can pause. Here's a long explanation of how to make that happen "Terminate busboy from reading more request with unpipe() and work queue". Removing listeners by itself is not enough, since there can be data in busboy's internal buffers and if you're doing anything asynchronous like piping or calling databases, the call to remove the listeners will be executed only after busboy has exhausted its internal buffer and fired more events. This causes errors that are hard to track as if busboy is firing ghost events.

The best way to signal the client to stop uploading is to send 413 status with 'Connection: close' header.

I'd also wrap the top-level function in an error handler like express-async-handler, so you can tidy up your code from catch blocks. This is to handle cases where busboy hasn't started yet, like the constructor throwing an error as you mentioned.

Then you'd have code like

const Busboy = require("busboy");
const PQueue = require("p-queue");
const express = require("express");
const handleAsync = require("express-async-handler");

const router = new express.Router();

router.post("/", handleAsync((req, res, next) => {
    const busboy = new Busboy({ headers: req.headers });
    const workQueue = new PQueue({ concurrency: 1 });

    function abort() {
      req.unpipe(busboy);
      workQueue.pause();
      if (!req.aborted) {
        res.set("Connection", "close");
        res.sendStatus(413);
      }
    }

    async function abortOnError(fn) {
      workQueue.add(async () => {
        try {
          await fn();
        } catch (e) {
          abort();
        }
      });
    }

    busboy.on("file", (fieldname, file, filename) => {
      abortOnError(() => {
        // my logic here...
      });
    });

    req.on("aborted", abort);
    busboy.on("error", abort);

    req.pipe(busboy);
  })
);

which is a mouthful, but it's the tradeoff between performance and readability.