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

asyncLocalStorage returns undefined if the request body is not empty (post, patch ...) #37207

Open
LoginovSO opened this issue Feb 3, 2021 · 13 comments
Labels
async_hooks Issues and PRs related to the async hooks subsystem.

Comments

@LoginovSO
Copy link

node v14.6.0

Additional information

returns undefined if the request body is not empty (post, patch ...)
when using async localStorage. run ()
but when using async localStorage.enter With() everything works fine

@LoginovSO
Copy link
Author

probems when the body is written in json format

@juanarbol
Copy link
Member

Hi @LoginovSO, thanks for opening this report!

could you please provide some code snippet to reproduce this behavior? That would be very appreciated.

@LoginovSO
Copy link
Author

const asyncLocalStorage = new async_hooks.AsyncLocalStorage();


@Module({
    providers: [
        {
            provide: ASYNC_STORAGE,
            useValue: asyncLocalStorage,
        }
    ],
})
export class SomeModule {}

async function bootstrap() {
  app.use((req, res, next) => {
    const asyncStorage = app.get(ASYNC_STORAGE);
....
    const store = new Map().set('store', {
      foo
    });
    asyncStorage.run(store, () => {
       next();
    });
  });
}

export class JwtStrategy extends PassportStrategy(Strategy) {
    constructor(
        @Inject(ASYNC_STORAGE) private readonly asyncStorage: AsyncLocalStorage<Map<string, Record<string, any>>>
    ) {}

    async validate(payload: IAuthPayload)
    {
     const store= this.asyncStorage.getStore()?.get('store'); // undefined

I hope you will understand

@targos targos added the async_hooks Issues and PRs related to the async hooks subsystem. label Aug 9, 2021
@targos
Copy link
Member

targos commented Aug 9, 2021

Is this still an issue with Node.js 14.17.4?

@nini
Copy link

nini commented Sep 10, 2021

I have a similar issue when using express and a body parser middleware. When body parser is executed, e.g. by matching content type (application/xml) of incoming request, then in the following controller, async store will be undefined.

@nini
Copy link

nini commented Sep 10, 2021

Btw. thanks for the workaround with enterWith() @LoginovSO.

@nini
Copy link

nini commented Sep 10, 2021

Working code sample below.
Use curl command as follows:

curl -XPOST localhost:3000/increment -H "Content-Type: application/xml" -d "<xml>test</xml"

code:

import * as express from "express";
import { NextFunction, Request, Response, Router, text } from "express";
import { AsyncLocalStorage } from "async_hooks";

const als: AsyncLocalStorage<Map<string, string>> = new AsyncLocalStorage();

class TestController {
    public increment(_request: Request<unknown, unknown, unknown>, response: Response, next: NextFunction): void {
        console.log("increment store");
        als.getStore()?.set("storeValue", "incremented");
        response.status(200).send("incremented");
        next();
    }
}
const router = Router();
const controller = new TestController();

// note text() middleware - it will break async store, unless fix is applied, see comment below
router.post("/increment", text({ type: "application/xml" }), controller.increment);

const server = express();
server.use((_req, _res, next) => {
    console.log("init async store");
    als.run(new Map(), next);

    //fix: comment line above, uncomment below
    // als.enterWith(new Map());
    // next();
});
server.use("", router);
server.use(async (_req, _res, _next) => {
    console.log("use async store value");
    const store = als.getStore();
    console.log({ store });
});

server.listen(3000);

@nini
Copy link

nini commented Sep 10, 2021

I suppose, this is related stream-utils/raw-body#71

@chill-cod3r
Copy link

The docs mention a caveat with using enterWith as opposed to run - is anyone in this thread knowledgeable about that?
https://nodejs.org/docs/latest-v14.x/api/async_hooks.html#async_hooks_asynclocalstorage_enterwith_store

I don't believe I fully understand why enterWith is referenced that way - something about possibly leaking context maybe?

@Flarna
Copy link
Member

Flarna commented Jan 14, 2022

@wolfejw86

The difference is that run has a clear start/end whereas enterWith doesn't have this. User may call exit or not.
consider following sample of an http server where several request listeners are attached:

const server = http.createServer();
server.on("request", (req, res) => {
  // this effects also the other listeners below and the whole code executed within node http module after `emit("request", req, res)` returned 
  als.enterWith(myStore1);
  // here comes your code
});
server.on("request", (req, res) => {
  // this effects only the code in given callback but not the other listener below nor node http internals
  als.run(() => {
  }, myStore2);
});
server.on("request", (req, res) => {
  // myStore1 is still active here, myStore2 is not
});

If run or enterWith is correct/better depends on the usecase. The main issue with enterWith is the limited scope and even if user tries to call exit at the right place it's easy that a place is forgotten e.g. if an exception is thrown,...

@chill-cod3r
Copy link

@Flarna Thanks for that - that confirms what I know about it except for one part- do you have to call exit manually? Say I only have one request handler listener, and I’m using enterWith at the start of each one. The contexts still appear to get cleaned up, there’s just the fact that the “previous context” is accessible by the next request until I start the next cycle of enterWith -> run right?

@Flarna
Copy link
Member

Flarna commented Jan 14, 2022

if calling exit is needed or not depends on the usecase. The active context stays active until

  • exit is called
  • enterWith is called to replace it
  • the async resource gets destroyed

When an async resource gets destroyed depends on the resource. e.g. process.nextTick(fn) creates a resource which is entered just before fn is called end destroyed after it returned. Other resources (e.g. a HTTP request) may have a longer lifetime as they may get triggered by more input data,...
Once enterWith is called the context is attached to this resource and whenever it gets scheduled to run some code the context is active.

I know that this is not that easy to understand. You could use async hooks to monitor init/enter/exit/destroy of resources in some experiment to get some better insight.

@JCMais
Copy link
Contributor

JCMais commented Jun 11, 2024

Did anything change about this or would it still be recommended to use enterWith when creating a async local storage with express using Node.js v20?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants