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

Node 16.2.0 with Jest break AsyncLocalStorage #49

Closed
2 tasks done
kibertoad opened this issue May 21, 2021 · 13 comments
Closed
2 tasks done

Node 16.2.0 with Jest break AsyncLocalStorage #49

kibertoad opened this issue May 21, 2021 · 13 comments

Comments

@kibertoad
Copy link
Member

kibertoad commented May 21, 2021

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure it has not already been reported

Fastify version

3.x.x

Node.js version

16.2.0

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

Linux, Windows, doesn't matter

Description

All tests are failing when being run on Node 16.2.0. Seems like all stored values are lost and are being resolved to undefined.

Considering that tests for https://github.com/kibertoad/asynchronous-local-storage are still passing with 16.2.0, looks like this particular plugin is doing something wrong.

Steps to Reproduce

Execute existing test suite on Node 16.2.0

Expected Behavior

Test suite passes.

@kibertoad
Copy link
Member Author

@puzpuzpuz Maybe you have any ideas what could be wrong? Wasn't nodejs/node#36394 supposed to be a non-breaking change?

@mcollina
Copy link
Member

I would recommend opening this on Node.js

@mcollina
Copy link
Member

@Qard can you take a look?

@Qard
Copy link

Qard commented May 22, 2021

I'll try to find some time to look at it soon. Just got vaccinated yesterday so I'm a bit out of commission today. 😷

@Qard
Copy link

Qard commented May 22, 2021

Looks like probably the same issue as was discovered in DataDog/dd-trace-js#1095, which seems to be only producible when running in Node.js 16.2.0 and jest. I'll try to find some time to copy/paste the tests into vanilla scripts without jest possibly tomorrow to verify.

@Qard
Copy link

Qard commented May 22, 2021

Got a bit of time to try it before bed and as I thought, it only happens within Jest. So Jest must be doing something different. Not sure what. 🤔

@puzpuzpuz
Copy link

@puzpuzpuz Maybe you have any ideas what could be wrong? Wasn't nodejs/node#36394 supposed to be a non-breaking change?

Yes, it supposed to be a non-breaking change. And I can also see test failures in https://github.com/puzpuzpuz/cls-rtracer + node v16.2.0 which uses jest.

@mcollina
Copy link
Member

mcollina commented May 22, 2021

The short term fix is to migrate off Jest. It's a good thing to do anyway.

(Jest does an absurd amount of monkeypatching of core, so any internal change is likely to break them).

@kibertoad
Copy link
Member Author

@mcollina Yes, I'm thinking about rewriting tests in TAP. The only concern that I have is that Jest is a very popular framework, and if fastify-request-context will start failing for people using it again in the future, and we don't have a quick way to repro it, that will be inconvenient. So I wonder if we should have also tests in Jest, that will be ignored for now.

@kibertoad
Copy link
Member Author

@puzpuzpuz Gotcha, thank you for the clarification!

@kibertoad kibertoad changed the title Node 16.2.0 breaks AsyncLocalStorage Node 16.2.0 with Jest break AsyncLocalStorage May 22, 2021
@targos
Copy link

targos commented May 23, 2021

I posted a reproduction of the issue that doesn't use Jest here: nodejs/node#38577 (comment)

@blimmer
Copy link

blimmer commented Jun 2, 2021

It looks like a fix should land in Node 16.3.0.

@kibertoad
Copy link
Member Author

Confirmed, 16.3.0 resolves the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants