davidbanham / express-async-errors

async/await support for ExpressJS
Other
900 stars 43 forks source link

Not working properly when using with multer upload #46

Open junioroo opened 1 year ago

junioroo commented 1 year ago

I've changed one of the tests to match the same approach I'm using in my application but it's not being handled:

const multer = require('multer');
const storage = multer.diskStorage({
  destination(req, file, cb) {
    cb(null, '/tmp/my-uploads');
  },
  filename(req, file, cb) {
    const uniqueSuffix = `${Date.now()}-${Math.round(Math.random() * 1E9)}`;
    cb(null, `${file.fieldname}-${uniqueSuffix}`);
  },
});

const uploader = multer({
  storage,
  preservePath: true,
  limits: { fileSize: 999 },
}).array('file');

it('propagates routes errors to error handler in Router with multer', () => {
    const app = express();
    const router = new Router();

    router.post('/test', async (req, res, next) => {
        uploader(req, res, (err) => {
          throw new Error('error');
      });
    });

    app.use('/upload', router);

    app.use((err, req, res, next) => {
      res.status(495);
      res.end();
    });

   return supertest(app)
      .post('/upload/test')
      .attach('file', `${__dirname}/.travis.yml`)
      .expect(495);
});

When I execute the test, I'm only receiving the crash "error" in the terminal, not the expected status from the handler!

Thanks for the help

code0monkey1 commented 1 year ago

Same ! I struggled 2 days to make sense of the problem ! The culprit was this package . While uploading a file , if you go and check If the file was uploaded or not ... there is a resulting unhandled promise rejection and the whole server crashes

code0monkey1 commented 1 year ago

@junioroo Well , I tried a few more packages to handle async errors , but unfortunately , they too could not catch the async errors thrown by multer . So , well I went ahead and added next to this particular controller that used multer , and kept the rest of the controllers the same ( no next )

//[+]2. Setting up multer function
const handleMultipartData = multer({
  storage,
  limits: { fileSize: 1000000 * 5 },
}).single('image'); // 5mb

//[+] Function to create a new product

// eslint-disable-next-line @typescript-eslint/require-await
const create = async (req: Request, res: Response, next: NextFunction) => {
  // Multipart form data

  // eslint-disable-next-line @typescript-eslint/no-misused-promises
  handleMultipartData(req, res, async (err) => {
    try {
      let product;

      if (err)
        return next(CustomErrorHandler.multerError('❌ Error at multer start'));

      if (!req.file) {
        return next(
          CustomErrorHandler.multerError(
            '❌ File Not Present in Multer Request'
          )
        );
      }

      const filePath = req.file.path;

      console.log('filePath ', filePath);

      //[+]validate the form data for product fileds

      //[-] You need to go to the `server.ts` file and apply the middleware to parse multipart form

      try {
        const body: unknown = await req.body;
        console.log('product body', JSON.stringify(body, null, 2));
        //[+] Extract product fields from the body and create a Product document
        const { name, price, size } = productValidator.parse(body);
        console.log(
          '🚀 ~ file: productController.ts:56 ~ handleMultipartData ~ const { name, price, size }:',
          name,
          price,
          size
        );

        product = await Product.create({
          name,
          price,
          size,
          image: filePath,
        });

        //[+]Delete the uploaded file in case of validation error
      } catch (err) {
        //[+] Return error res in case error
        fs.unlink(`${APP_ROOT}/${filePath}`, (err) => {
          if (err)
            throw CustomErrorHandler.multerError('Could not delete file');
          else console.log('✅ Uploaded file deleted');
        });
        return next(CustomErrorHandler.multerError((err as Error).message));
      }

      return res.status(201).json(product);
    } catch (err) {
      next(err);
    }
  });
UltimateGG commented 11 months ago

This is not just for multer, just tested and its when an inner function throws an unhandled rejection.

app.get('/async', async (req: Request, res: Response) => {
  async function test() {
    throw new Error('ASYNC test;' + Date.now());
  }
  test();
});

Crashes the app. Interestingly, adding await infront of the call to test() fixes the error. I think something similar is happening here.

I would think the easiest workaround is to not use nesting and use a cleaner syntax anyways (await).

Instead of using the callback, see if you can use await