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

Unhandled Rejection: deno run -A npm:vitest --run (tinypool) #24796

Closed
gvelim opened this issue Jul 30, 2024 · 1 comment · Fixed by #24763
Closed

Unhandled Rejection: deno run -A npm:vitest --run (tinypool) #24796

gvelim opened this issue Jul 30, 2024 · 1 comment · Fixed by #24763
Labels
bug Something isn't working correctly node compat

Comments

@gvelim
Copy link

gvelim commented Jul 30, 2024

Issue

When I run deno run -A npm:vitest --run I get the following response

 RUN  v2.0.4 /Users/xxxxx/development/advent_js

error: unexpected argument '--conditions' found

  tip: to pass '--conditions' as a value, use '-- --conditions'

Usage: deno run --node-modules-dir[=<node-modules-dir>] --allow-all [SCRIPT_ARG]...

For more information, try '--help'.

error: unexpected argument '--conditions' found

  tip: to pass '--conditions' as a value, use '-- --conditions'

Usage: deno run --node-modules-dir[=<node-modules-dir>] --allow-all [SCRIPT_ARG]...

For more information, try '--help'.


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
TypeError: serde_v8 error: invalid type; expected: buffer, got: Int32Array
 ❯ ChildProcess.target.send ext:deno_node/internal/child_process.ts:993:5
 ❯ ProcessWorker.send node_modules/.deno/tinypool@1.0.0/node_modules/tinypool/dist/index.js:175:20
 ❯ ProcessWorker.postMessage node_modules/.deno/tinypool@1.0.0/node_modules/tinypool/dist/index.js:194:17
 ❯ ThreadPool._addNewWorker node_modules/.deno/tinypool@1.0.0/node_modules/tinypool/dist/index.js:712:12
 ❯ ThreadPool._ensureMinimumWorkers node_modules/.deno/tinypool@1.0.0/node_modules/tinypool/dist/index.js:651:12
 ❯ new ThreadPool node_modules/.deno/tinypool@1.0.0/node_modules/tinypool/dist/index.js:636:10
 ❯ new Tinypool node_modules/.deno/tinypool@1.0.0/node_modules/tinypool/dist/index.js:966:31
 ❯ createForksPool node_modules/.deno/vitest@2.0.4/node_modules/vitest/dist/vendor/cli-api.CTkP2Ier.js:8402:16
 ❯ forks node_modules/.deno/vitest@2.0.4/node_modules/vitest/dist/vendor/cli-api.CTkP2Ier.js:9351:20
 ❯ node_modules/.deno/vitest@2.0.4/node_modules/vitest/dist/vendor/cli-api.CTkP2Ier.js:9378:41

Workaround 1

When I run deno run -A npm:vitest --pool=threads then I get a number of tests hanging. Not always the same tests hang

Workaround 2

When I run deno run -A npm:vitest --pool=threads --no-file-parallelism then everything runs with no issues

Installed versions

deno --version
deno 1.45.4 (release, x86_64-apple-darwin)
v8 12.7.224.13
typescript 5.5.2

deno run -A npm:vitest --version
vitest/2.0.4 darwin-x64 node-v20.11.1

@nathanwhit not sure to what extend this might be related to the PR you are working on

@nathanwhit
Copy link
Member

not sure to what extend this might be related to the PR you are working on

Very much related! This issue is actually what got me started on that PR 😄 This should be fixed once it merges.

crowlKats pushed a commit that referenced this issue Jul 31, 2024
Fixes #24756. Fixes
#24796.

This also gets vitest working when using
[`--pool=forks`](https://vitest.dev/guide/improving-performance#pool)
(which is the default as of vitest 2.0). Ref
#23882.

---

This PR resolves a handful of issues with child_process IPC. In
particular:

- We didn't support sending typed array views over IPC
- Opening an IPC channel resulted in the event loop never exiting
- Sending a `null` over IPC would terminate the channel
- There was some UB in the read implementation (transmuting an `&[u8]`
to `&mut [u8]`)
- The `send` method wasn't returning anything, so there was no way to
signal backpressure (this also resulted in the benchmark
`child_process_ipc.mjs` being misleading, as it tried to respect
backpressure. That gave node much worse results at larger message sizes,
and gave us much worse results at smaller message sizes).
- We weren't setting up the `channel` property on the `process` global
(or on the `ChildProcess` object), and also didn't have a way to
ref/unref the channel
- Calling `kill` multiple times (or disconnecting the channel, then
calling kill) would throw an error
- Node couldn't spawn a deno subprocess and communicate with it over IPC

(cherry picked from commit cd59fc5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants