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 and res.locals unexpected behavior starting in 13.4.3-canary.3 #53466

Open
1 task done
helloitsjoe opened this issue Aug 1, 2023 · 13 comments
Open
1 task done
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team.

Comments

@helloitsjoe
Copy link

helloitsjoe commented Aug 1, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 20.6.0: Thu Mar  9 20:39:26 PST 2023; root:xnu-7195.141.49.700.6~1/RELEASE_X86_64
    Binaries:
      Node: 20.2.0
      npm: 9.6.6
      Yarn: 3.6.1
      pnpm: N/A
    Relevant Packages:
      next: 13.4.12
      eslint-config-next: 13.4.12
      react: 18.2.0
      react-dom: 18.2.0
      typescript: N/A
    Next.js Config:
      output: N/A

NOTE: 13.4.12 is the most recent version I could run, all 13.4.13 canaries are crashing with the following error:

Failed to proxy http://127.0.0.1:0/about Error: connect EADDRNOTAVAIL 127.0.0.1 - Local (0.0.0.0:57514)

Which area(s) of Next.js are affected? (leave empty if unsure)

App Router

Link to the code that reproduces this issue or a replay of the bug

https://github.com/helloitsjoe/next-custom-server-repro

To Reproduce

Please see the reproduction repo, it has a full example of the setup described here and a more detailed description in the README:

  1. Create a custom server
  2. Create an instance of AsyncLocalStorage, and call .run() in custom server middleware
  3. Attach data to res.locals in custom server middleware
  4. From a file in the app directory, try to access the store instance with .getStore(), see that it's undefined
  5. From getServerSideProps in a file in the pages directory, try to access the store instance with .getStore(), see that it's undefined
  6. From getServerSideProps in a file in the pages directory, try to access res.locals, see that it's also undefined

Describe the Bug

The AsyncLocalStorage store is undefined in application code in the app directory, and res.locals is undefined in the pages directory (in getServerSideProps), even though they are available in the custom server at request time. This behavior started in next@13.4.3-canary.3.

Expected Behavior

I expect to have access to a globally defined AsyncLocalStorage store server-side in components in the app directory, and access to both the store and res.locals in getServerSideProps in the pages directory, without setting customServer: false. This is the behavior in next@13.4.2.

Which browser are you using? (if relevant)

115.0.5790.114 (Official Build) (x86_64)

How are you deploying your application? (if relevant)

No response

NEXT-1489

@helloitsjoe helloitsjoe added the bug Issue was opened via the bug report template. label Aug 1, 2023
@balazsorban44 balazsorban44 added the linear: next Confirmed issue that is tracked by the Next.js team. label Aug 2, 2023
@shaunwarman
Copy link

shaunwarman commented Sep 20, 2023

@helloitsjoe - is this a safe option to cross boundaries of custom server and provide that context to server component pre-render? Any worries of interleaving requests and mixing up user data here? cc @coltonehrman

This reminds me of the issues that came with https://github.com/othiym23/node-continuation-local-storage and not explicitly passing context object through middleware.

@dhalbrook
Copy link

Still seeing this on 13.4.18

@tatethurston
Copy link

I ran into this trying to use AsyncLocalStorage. The instance of AsyncLocalStorage needs to be shared across Next runtime contexts using globalThis (seen in the linked repro case above).

Did something change with the handling of globalThis across next contexts in 13.4.2?

@stychu
Copy link

stychu commented Oct 27, 2023

@tatethurston Did you find it working? I face the same issue with custom express server that the asynclocalstorage is not visible on the NextJs runtime context but works no problem with Express server context. I wanted to setup this for request tracing in logs but logs that I put in nextjs API or RSC don't share the asynclocalstorage that is initialized from custom express server ;(

@stychu
Copy link

stychu commented Nov 3, 2023

This works for me with latest Nextjs 13/14

/* eslint-disable no-var */

// TODO Look into AsyncContext instead AsyncLocalStorage
//  for performance improvements once its implemented in javascript runtime
//  https://github.com/tc39/proposal-async-context
/*
 * Set tracing store on globalThis so asyncLocalStorage
 * can work in both NextJs and Express runtime contexts
 */
import { AsyncLocalStorage } from 'node:async_hooks';

import type { NextFunction } from 'express';

declare global {
  var tracingRequestGlobalStore: AsyncLocalStorage<Tracing>;
}

type Tracing = {
  requestTraceId: string;
  persistentTraceId: string;
};

if (!globalThis.tracingRequestGlobalStore) {
  globalThis.tracingRequestGlobalStore = new AsyncLocalStorage<Tracing>();
}

export const getTracingRequestStore = (): Tracing | undefined =>
  globalThis.tracingRequestGlobalStore.getStore();

export const setTracingRequestStore = (
  tracing: Tracing,
  next: NextFunction,
) => {
  globalThis.tracingRequestGlobalStore.run(tracing, next);
};

@tatethurston
Copy link

@stychu yes in next versions prior to 13.4.2

@stychu
Copy link

stychu commented Nov 6, 2023

@tatethurston Im using "next": "14.0.2-canary.10", and its working also on 13.5.6 it worked for me too

@shaunwarman
Copy link

@coltonehrman

@coltonehrman
Copy link
Contributor

@tatethurston Im using "next": "14.0.2-canary.10", and its working also on 13.5.6 it worked for me too

Do you have a repo to reproduce a working example? I can't seem to get my setup working on either of these versions

@stychu
Copy link

stychu commented May 9, 2024

@helloitsjoe @coltonehrman @tatethurston I completely forgot about replying to the issue. I apologise guys. Dunno if you found the solution or ditched this completely but your issue is within the start-server.js script.

If you upgrade to the latest nextjs version 14.2.3 and change the server script to this it should work.

const { createServer } = require("http");
const next = require("next");
const express = require("express");
const { getStore, run } = require("./async-store");

console.log("start-server pid", process.pid);

const app = express();

const dev = process.env.NODE_ENV !== "production";
const setCustomServerFalse = process.env.SET_CUSTOM_SERVER_FALSE === "true";

console.log("setCustomServerFalse", setCustomServerFalse);

const nextApp = next({
  dev,
  // If customServer is not explicitly set to false, don't include it at all
  ...(setCustomServerFalse ? { customServer: false } : null),
});
const handle = nextApp.getRequestHandler();

nextApp.prepare().then(() => {
  console.log("NextApp is ready");

// Middleware to set up AsyncLocalStorage for passing context around
  app.use((req, res, next) => {
    const store = getStore();
    // Create a context for the request.
    // `run()` sets the AsyncLocalStorage instance's `enabled` to true
    run(new Map(), next);
  });

// Middleware to set up res.locals for passing context around
  app.use((req, res, next) => {
    res.locals.foo = "bar";
    next();
  });

  app.get("*", (req,res,next) => {
    handle(req,res).catch(next)
  });

  createServer(app).listen(3000, () => {
    console.log("listening on http://localhost:3000");
  });
});
Screenshot 2024-05-09 at 12 25 22 Screenshot 2024-05-09 at 12 25 28

The custom server setup need to be done within the nextapp prepare handler plus getRequestHandler should be called explicitly with req,res instead of passing it as a function to the express handler.

@acrognale
Copy link

The globalThis method as mentioned by @stychu works for me with Next 14.2.3

@joaquinbentancor
Copy link

I'm trying to wrap the middleware with AsyncLocalStorage to have a trace id in every request, this works fine at middleware level but when I try to retrieve the context info in a Route Handler, the context is undefined. Could be a bind problem? How can I tackle the problem?

@valourus
Copy link

The globalThis method worked for me on next 15.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team.
Projects
None yet
Development

No branches or pull requests

10 participants