expressjs / multer

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

CLS Context is lost after using multer middleware #814

Open josencv opened 4 years ago

josencv commented 4 years ago

Hello! I'm using the cls-hooked library, which let's me use continuous local storage in my express app. After I use the multer.file middleware, my context is lost.

I patched the middleware using the solution proposed here and now it works fine. I'm letting you know just in case you want to fix it.

Cheers,

Additional info Node version: 10.15.3 Express version: 4.16.3 Multer version: 1.4.2 CLS Hooked version: 4.2.2 OS: Ubuntu 18.04.3 LTS

Tchoupinax commented 3 years ago

Hello,

I have the same issue with the object AsyncLocalStorage (from 'async_hooks'), the context is lost on the route which has the multer middleware.

Edit: https://medium.com/@theekshanawj/nodejs-using-multer-and-cls-hooked-together-a00decbebab6 fixes my problem ^^

grzegorzjudas commented 3 years ago

I struggled for a long time with the same issue, but I was able to go around it by passing all necessary middlewares with the following notation:

async function uploadMiddleware (req: Request, res: Response, next: NextFunction) {
    try {
        await new Promise<void>((resolve, reject) => {
            multer({
                // ...
            }).array('files')(req, res, (error: any) => {
                if (error) return reject(error);

                return resolve();
            });
        });

        next();
    } catch (error) {
        next(error);
    }
}

server.get('/endpoint', uploadMiddleware, controller);

So basically what I'm doing is, I'm running the multer middleware in-directly by wrapping it in another middleware that handles context correctly and then, at correct time, continuing to next middlewares/controller by calling next().

Edit: just noticed it's basically the same solution as in the aforementioned medium.com article. So... yay me :) having it directly here may help people stumbling here on this problem.

ajhyndman commented 2 years ago

I just thought I'd share a cheap user-space workaround that my team came up with, in case it's helpful for anyone else.

const { AsyncResource } = require('node:async_hooks')

/**
 * Ensure that an express middleware respects async local storage context.
 *
 * Accepts an express middleware and returns a copy of that middleware with the
 * next() call explicitly bound to the current AsyncResource context.
 *
 * When working with native promises or async/await syntax, Node can
 * automatically forward any existing async resource to nested async or promise
 * invocations. However, when working with nested callback-passing patterns
 * (such as occurs in an express middleware stack) Node cannot know whether the
 * function being passed around should be bound to the current resource.
 *
 * Express may update their plugin architecture to resolve this in a future
 * version. Middleware developers can also update their implementations to
 * ensure backward-compatible support for AsyncLocalStorage. This function
 * allows us to work around the issue for current versions of express and
 * plugins that have not yet been updated.
 */
function ensureAsyncContext (middleware) {
  return (req, res, next) => middleware(req, res, AsyncResource.bind(next))
}

Usage:

router.post(
  '/upload',
  // ... other plugins
  ensureAsyncContext(multerUpload.single('myUpload')),
  async (req, res, next) => {
    // ...
  },
);
GutoMartins19 commented 1 year ago

I just thought I'd share a cheap user-space workaround that my team came up with, in case it's helpful for anyone else.

const { AsyncResource } = require('node:async_hooks')

/**
 * Ensure that an express middleware respects async local storage context.
 *
 * Accepts an express middleware and returns a copy of that middleware with the
 * next() call explicitly bound to the current AsyncResource context.
 *
 * When working with native promises or async/await syntax, Node can
 * automatically forward any existing async resource to nested async or promise
 * invocations. However, when working with nested callback-passing patterns
 * (such as occurs in an express middleware stack) Node cannot know whether the
 * function being passed around should be bound to the current resource.
 *
 * Express may update their plugin architecture to resolve this in a future
 * version. Middleware developers can also update their implementations to
 * ensure backward-compatible support for AsyncLocalStorage. This function
 * allows us to work around the issue for current versions of express and
 * plugins that have not yet been updated.
 */
function ensureAsyncContext (middleware) {
  return (req, res, next) => middleware(req, res, AsyncResource.bind(next))
}

Usage:

router.post(
  '/upload',
  // ... other plugins
  ensureAsyncContext(multerUpload.single('myUpload')),
  async (req, res, next) => {
    // ...
  },
);

Thanks for sharing!