expressjs / multer

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

Request not finish when recieve an error #1247

Open pullmann4rent opened 8 months ago

pullmann4rent commented 8 months ago

I use multer for uploading files and if I upload an unexisting file or with a different file name then I get this

MulterError: Unexpected field

This is good that I catch the error but I can not send anymore request until I restart. Why ?

I use all of my files try catch and when I get an error I never got this problem. Only when it goes to my custom error handler:

export const app = express();

const upload = multer({ 
 limits: {
 fileSize: 10 * 1024 * 1024,
  },  
 fileFilter: (_req, file, cb) => {
 if (!allowedFileMimetypeAndExtensions.includes(file.mimetype)) {
 return cb(new Error('Diese Bilddatei ist nicht erlaubt.'));
    }

 cb(null, true)
  }
});

const redisStore = new RedisStorage({ client: redis });

// The request handler must be the first middleware on the app
app.use(Sentry.Handlers.requestHandler());

// TracingHandler creates a trace for every incoming request
app.use(Sentry.Handlers.tracingHandler());

app.use(morgan('dev'));

app.use(express.urlencoded({extended: true}));
app.use(express.json());

//Reduce Fingerprint
app.disable('x-powered-by');

app.use(cors());
app.use(helmet());

app.post('/api/upload/images', [upload.array('image', 10)], async (req: Request, res: Response) => {
 try {
 console.log(req.file);
 console.log(req.files);
 return res.status(201).json({});
  } catch(e) {
 console.log(e);
 Sentry.captureException(e);
 return res.status(500).json({
 message: e
    });
  }
});

// The error handler must be registered before any other error middleware and after all controllers
app.use(Sentry.Handlers.errorHandler());

// Custom Error Handler HERE I CATCH THE ERROR
app.use(errorHandler);
app.listen(3000, () => {
 console.log('SERVER LISTEN ON PORT 3000');
});

And here is my custom error handler

import * as Sentry from '@sentry/node';
import { NextFunction, Request, Response } from 'express';

export const errorHandler = (err: Error, _req: Request, res: Response, _next: NextFunction) => {
  Sentry.captureException(err);
  console.log(`Errorhandler: ${err}`);
  return res.status(500).json({error: 'Etwas ist schief gelaufen.'});
};
joeyguerra commented 8 months ago

@pullmann4rent I don't see anything obvious in the code. Multer calls the callback with an Error object can you see in your code which callback is getting that error object?

devslimbr commented 7 months ago

"I'm facing the same issue. It happens when the multipart/form-data field contains an expected object with a different name. If I set it as 'file' and send it as 'item,' it will respond that it doesn't exist. The problem is that it sends a 500 error to the server and gets stuck in a loop. If I use multer's handleError, the error will be caught only once and the entire system will be stuck until a restart is necessary."

joeyguerra commented 7 months ago

Can we get a test case to reproduce?

devslimbr commented 7 months ago

To fix the problem, I had to change upload.array to upload.any and do the validations internally

1.) The upload must be a multer.array('name', X). X can be any number

2.) In the request, enter a name other than "name". It will give the multer error, but it gets stuck in the infinite loop. Ex: field "image" to "images"

In other words, if I'm creating an api and the user uses a name other than the one required, the multer triggers the error and gets stuck. The solution is to restart the application.

joeyguerra commented 7 months ago

How does the user submit a different name other than the one required?

devslimbr commented 7 months ago

I was making an image upload API for AWS S3 that accepts an array of fields of type 'multipart/form-data'.

However, in my request I expect the field to be file, that is, a file array.

If the user makes a mistake in the field and sends files, the multer gives an error 500 and warns "Unexpected field". However, I don't want it to give a 500 error, but rather another custom error.

If I use multer's handleError, I can catch the exception and send a custom error, but it stays in an infinite loop until the application crashes

joeyguerra commented 7 months ago

Is the user typing in a textbox the word "files"? Is this test case the same scenario as yours?

devslimbr commented 7 months ago

Example

Captura de tela 2024-04-18 195529

Expectative: file User: User change to files, error 500 looping

joeyguerra commented 7 months ago

Thank you for the additional information. It looks like there is a unit test for the unexpected field name situation.

My current guess is that the "looping" behavior is caused by additional code and how it interacts with the request/response handling code. Would you be able to post the code that's handling errors?

devslimbr commented 7 months ago

Inside my try cath there is my upload logic.

app.post('/upload-files', upload.array('file', 6), handleMulterError, async (req: Request, res: Response, next: NextFunction) => {
    try {

        .....

    } catch (error) {

    .....
    }
});

function handleMulterError(err: any, req: Request, res: Response, next: NextFunction) {
    if (err instanceof multer.MulterError) {
        if (err.code === 'LIMIT_UNEXPECTED_FILE') {
            return res.status(403).send({
                success: false,
                error: 'No files uploaded',
            });
        }
    }
    next(err);
}

The problem is when the multer error occurs. I can give the 403 error, but it will be stuck in a loop waiting for request. I've already tried removing next(err) and it gives the same error.

To solve the problem, I replaced the array with any, and inside I did the logic of going through the array and seeing if it was the correct field, giving my custom error without crashing the app

joeyguerra commented 7 months ago

Can you try "using" the error handler after the app.post handler registration? Something like this test.