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

[BUG] Test Deadlock - Node v18.19.0 #992

Open
3 tasks done
pmarchini opened this issue Jan 3, 2024 · 4 comments
Open
3 tasks done

[BUG] Test Deadlock - Node v18.19.0 #992

pmarchini opened this issue Jan 3, 2024 · 4 comments
Labels
bug something not go good

Comments

@pmarchini
Copy link

pmarchini commented Jan 3, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Have you read the CONTRIBUTING guide on posting bugs, and CODE_OF_CONDUCT?

  • yes I read the things

This issue exists in the latest tap version

  • I am using the latest tap

Description

Hey!

We recently upgraded a large codebase from Node version 18.18.2 to 18.19.0 and encountered an odd issue. Some of our test suites are randomly getting stuck until a timeout forces them to terminate. We have confirmed that no other dependencies were altered during this process.

I've included a repository with a reproducible example.

To replicate the problem, simply run the 'make' command. After a while, you'll notice that the test suite gets stuck, waiting for the timeout.

While attempting to debug the Node process in this stuck state, I discovered that the system seems to be looping at this particular line: https://github.com/tapjs/async-hook-domain/blob/9885b0d917abd053abf619ba8d37a9a57397ba0a/src/index.ts#L169.

Specifically, the destroy function is receiving a series of ids that are not present in domains (which typically contains only one id). Interestingly, when I manually change the id passed to domains.get through the debugger, the test suite completes successfully.

Additionally, it's worth noting that when we roll back to version 18.18.2, this issue seems to be resolved.

Reproduction

https://github.com/pmarchini/tap-fastify-repro

Environment

tap: 18.6.1
"@tapjs/config": 2.4.14
"@tapjs/core": 1.4.6
"@tapjs/run": 1.4.16
"@tapjs/stack": 1.2.7
"@tapjs/test": 1.3.17
tap-parser: 15.3.1
tap-yaml: 2.2.1
tcompare: 6.4.5
plugins:
  "@tapjs/after": 1.1.17
  "@tapjs/after-each": 1.1.17
  "@tapjs/asserts": 1.1.17
  "@tapjs/before": 1.1.17
  "@tapjs/before-each": 1.1.17
  "@tapjs/filter": 1.2.17
  "@tapjs/fixture": 1.2.17
  "@tapjs/intercept": 1.2.17
  "@tapjs/mock": 1.2.15
  "@tapjs/node-serialize": 1.2.6
  "@tapjs/snapshot": 1.2.17
  "@tapjs/spawn": 1.1.17
  "@tapjs/stdin": 1.1.17
  "@tapjs/typescript": 1.3.6
  "@tapjs/worker": 1.1.17
# vim: set filetype=yaml :

# from package.json
allow-incomplete-coverage: true

# env, cli, and defaults
color: true
coverage-report:
  - text
exclude:
  - "**/@(fixture*(s)|dist)/**"
include:
  - "**/@(test?(s)|__test?(s)__)/**/*.@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/*.@(test?(s)|spec).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/test?(s).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
jobs: 6
reporter: base
snapshot-clean-cwd: true
timeout: 30
@tapjs/after
@tapjs/after-each
@tapjs/asserts
@tapjs/before
@tapjs/before-each
@tapjs/filter
@tapjs/fixture
@tapjs/intercept
@tapjs/mock
@tapjs/node-serialize
@tapjs/snapshot
@tapjs/spawn
@tapjs/stdin
@tapjs/typescript
@tapjs/worker

EDIT: I've checked, and the async-hooks that I see during debugging (while stuck) are probably related to the timer.

@pmarchini pmarchini added the bug something not go good label Jan 3, 2024
@piotr-cz
Copy link

I feel like this might related to an issue of ts-node which doesn't play nicely with node 18.19.x and 20.x: TypeStrong/ts-node#1997

@pmarchini
Copy link
Author

I feel like this might related to an issue of ts-node which doesn't play nicely with node 18.19.x and 20.x: TypeStrong/ts-node#1997

Hey,

Thanks for your answer,
If it could be helpful I've tried also disabling ts plugin (the repro I've submitted doesn't contain ts) and the problem persists.

@piotr-cz
Copy link

@pmarchini
There is a worakound to run tests with tsx instead of ts-node: #976 (comment)

@isaacs
Copy link
Member

isaacs commented Jan 18, 2024

I'll be switching to https://github.com/tapjs/tsimp instead of ts-node, but there's still a few kinks to comb out. In the meantime, yes, tsx is an option.

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

No branches or pull requests

3 participants