expressjs / multer

Node.js middleware for handling `multipart/form-data`.
MIT License
11.61k stars 1.06k forks source link

Multer & Express.js File Upload request hanged (never ending pending) #1218

Open agussetyar opened 1 year ago

agussetyar commented 1 year ago

I'm working with very simple file upload feature using Express.js & Multer.

This is my multer middleware code:

const path = require('path')
const multer = require('multer')

const storage = multer.diskStorage({
  destination: function (req, file, cb) {
    // Set the destination folder for uploaded files
    cb(null, './public/uploads/')
  },
  filename: function (req, file, cb) {
    // Generate a unique filename to avoid naming conflicts
    const uniqueSuffix = Date.now() + '-' + Math.round(Math.random() * 1e9)
    const originalFileExtension = path.extname(file.originalname)
    const filename = file.fieldname + '-' + uniqueSuffix + originalFileExtension

    cb(null, filename)
  },
})

const upload = multer({
  storage: storage,
  limits: { fileSize: 100 * 1024 * 1024 },
  fileFilter: function (req, file, cb) {
    const allowedTypes = /pdf|xls|zip|rar|jpg/
    const fileType = path.extname(file.originalname).toLowerCase()

    if (allowedTypes.test(fileType) === false) {
      const err = new Error('File extensions is not allowed.')
      err.status = 500
      err.name = 'FileValidationError'

      cb(err, false)
    }

    cb(null, true)
  },
})

module.exports = upload

This is my error handler middleware:

// wrap unathorized error
router.use((err, req, res, next) => {
  if (['UnauthorizedError', 'FileValidationError'].includes(err.name)) {
    return res.status(err.status).json({
      error: true,
      code: err.status,
      message: err.message,
    })
  }

  next(err)
})

When I hit this endpoint from Frontend (React.js) using Axios and FormData. I got never ending 'pending' request and I dont know why this is happen with this simple code. The expected results is custom JSON response should be returned. pending

After a minutes searching, I found that if I remove the 'FileValidationError' from error handler logic or using this code:

if (err instanceof multer.MulterError) {}

The request is not pending and it returned response the Error() instance. catchError

But, this behavior only happens in the Frontend side. I tested with Postman and the request looks fine.

Can someone tell me If I do something wrong in this situation?

baselshoban commented 1 year ago

But, this behavior only happens in the Frontend side. I tested with Postman and the request looks fine.

@agussetyar I guess you need to add the header Accept: application/json in your frontend request in order to receive a JSON response

agussetyar commented 1 year ago

But, this behavior only happens in the Frontend side. I tested with Postman and the request looks fine.

@agussetyar I guess you need to add the header Accept: application/json in your frontend request in order to receive a JSON response

Already tried this approach, but also not working in my cases. So far, I just return the default throw exception without any customization into JSON format.

Hongdaesik commented 1 year ago

fileFilter( req, bile, cb ) { cb( new Error( 'error' ), false ) }

Processing works fine on the web, but when an application uses network communication, a timeout occurs in the Pending state.

We have confirmed that the busboy.on('close') event does not fall into the Pending state.

I corrected the code as below so that the storage._handleFile handled the error.

/lib/make-middleware.js: 116

if (err) {
  appender.removePlaceholder(placeholder)
  return abortWithError(err)
}

if (!includeFile) {
  appender.removePlaceholder(placeholder)
  return fileStream.resume()
}

var aborting = false
pendingWrites.increment()
var aborting = false
pendingWrites.increment()

if (err||!includeFile) {
  aborting = true
  abortWithError(err)
}
pwliuab commented 2 months ago

I have encountered this issue as well. It can be replicated even when I use Postman. After using @Hongdaesik 's fix, it works now.

pwliuab commented 2 months ago

I think this issue is due to the empty array of uploadedFiles, which causes the remove callback to not be triggered.

pwliuab commented 2 months ago

@Hongdaesik I'm curious about why this bug block exactly the first request after the filter file error. before using your fix, the busboy status is _complete: false. But I'm not sure why busboy blocks other unrelated node js requests

pwliuab commented 2 months ago

original code: _complete: false, _hparser: null,

after applying the fix: _fileEndsLeft: 0, _fileStream: null, _complete: true,

pwliuab commented 2 months ago

busyboy _bparser before the fix: image

pwliuab commented 2 months ago

for those who are waiting for merge, you can apply this fix in application code:

when there is a filefilter error, instead of passing it to the multer callback, you could flag it in req: 
                    req.uploadError = new Error("filefilter error");
                    cb(null, false);
----
then handle it by using your customize middleware func,
    const errorHandler = (req, res, next) => {
        if (req.uploadError) {
            return next(req.uploadError);
        }
        next();
    };

 const overriddenMulter = Object.keys(Object.getPrototypeOf(multerMiddleWare)).reduce(
        (fncMapper, fncName) => {
            if (typeof multerMiddleWare[fncName] !== "function") {
                return fncMapper;
            }
            fncMapper[fncName] = (...key) => [
                multerMiddleWare[fncName](...key),
                errorHandler,
            ];
            return fncMapper;
        },
        {}
    );

e.g.
route.post("/", overriddenMulter.single("image"), yourcontrollerFunc);
Hongdaesik commented 2 months ago

@pwliuab I'll check it out in more detail and make further corrections when I have time! thx!

pwliuab commented 2 months ago

@Hongdaesik thanks, I will investigate the detail as well, seems interesting.