Search code examples
javascriptnode.jsexpresspromisebusboy

ExpressJS apparent race condition between Promise and EventEmitter


I have a NodeJS/Express web application that allows the user to upload a file, which I then parse using connect-busboy save to my database using Sequelize. Once that's done, I want to redirect the user to a given page. But Express is returning a status of 404 before my Promise resolves, even though I'm never calling next(), which I thought was mandatory in order to call the next handler in the middleware chain and thus result in a 404.

This is my code so far:

function uploadFormFile(req, res, next) {
   var documentInstanceID = req.params.documentInstanceID;
   // set up an object to hold my data
   var data = {
    file: null,
    documentDate: null,
    mimeType: null
   };
   // call the busboy middleware explicitly 
   // EDIT: this turned out to be the problem... of course this calls next()
   // removing this line and moving it to an app.use() made everything work as expected
   busboy(req, res, next);
   req.pipe(req.busboy);
   req.busboy.on('file', function (fieldName, file, fileName, encoding, mimeType) {
    var fileData = [];
    data.mimeType = mimeType;
    file.on('data', function (chunk) {
        fileData.push(chunk);
    });
    file.on('end', function () {
        data.file = Buffer.concat(fileData);
    });
   });
   req.busboy.on('finish', function () {
    // api methods return promises from Sequelize
    api.querySingle('DocumentInstance', ['Definition'], null, { DocumentInstanceID: documentInstanceID })
        .then(function (documentInstance) {
        documentInstance.RawFileData = data.file;
        documentInstance.FileMimeType = data.mimeType;
        // chaining promise
        return api.save(documentInstance);
       }).then(function () {
        res.redirect('/app/page');
       });
   });
}

I can confirm that my data is being persisted correctly. But due to the race condition, the web page says 'can't POST' due to the 404 status being returned by Express, and the res.redirect is failing with an error setting the headers because it's trying to redirect after the 404 has been sent.

Can anyone help me figure out why Express is returning the 404?


Solution

  • The problem is coming from your internal call to busboy inside your handler. Rather than it executing and simply returning control to your handler, it would be calling the next which is passed to it before it returns control. So you code after the busboy call does execute, but the request has already advanced past that point.

    In cases in which you want some middleware to only be executed for certain requests, you can chain middleware into those requests, such as:

    router.post('/upload',busboy,uploadFromFile)
    

    You can also separate them with .use() such as:

    router.use('/upload', busboy);
    router.post('/upload', uploadFromFile);
    

    Either of the above will chain the middleware in the way you intended. In the case of .use() the middleware would also be applied to any applicable .METHOD() as Express refers to it in their documentation.

    Also, note that you can pass in an arbitrary number of middleware this way, either as separate parameters or as arrays of middleware functions, such as:

    router.post('/example', preflightCheck, logSomeStuff, theMainHandler);
    // or
    router.post('example', [ preflightCheck,logSomeStuff ], theMainHandler);
    

    The execution behavior of either of the above examples will be equivalent. Speaking only for myself and not suggesting it is a best practice, I normally only use the array-based addition of middleware if i am building the middleware list at runtime.

    Good luck with it. I hope you enjoy using Express as much as I have.