Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
agussetyar opened this issue Jun 30, 2023 · 11 comments
Open

Comments

@agussetyar
Copy link

agussetyar commented Jun 30, 2023

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
Copy link

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
Copy link
Author

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
Copy link

Hongdaesik commented Sep 21, 2023

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
Copy link

pwliuab commented Aug 18, 2024

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
Copy link

pwliuab commented Aug 18, 2024

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

@pwliuab
Copy link

pwliuab commented Aug 19, 2024

@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
Copy link

pwliuab commented Aug 19, 2024

original code:
_complete: false,
_hparser: null,

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

@pwliuab
Copy link

pwliuab commented Aug 19, 2024

busyboy _bparser before the fix:
image

@pwliuab
Copy link

pwliuab commented Aug 19, 2024

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
Copy link

Hongdaesik commented Aug 20, 2024

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

@pwliuab
Copy link

pwliuab commented Aug 20, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants