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
fix: unhandled error handler #876
Conversation
✔️ Deploy Preview for vitest-dev ready! 🔨 Explore the source changes: 292a59e 🔍 Inspect the deploy log: https://app.netlify.com/sites/vitest-dev/deploys/621d7489d85df800073f9ece 😎 Browse the preview: https://deploy-preview-876--vitest-dev.netlify.app |
31631ec
to
73797ef
Compare
package.json
Outdated
@@ -21,7 +21,7 @@ | |||
"test": "vitest --api -r test/core", | |||
"test:run": "vitest run -r test/core", | |||
"test:all": "cross-env CI=true pnpm -r --stream --filter !@vitest/monorepo run test -- --allowOnly", | |||
"test:ci": "cross-env CI=true pnpm -r --stream --filter !@vitest/monorepo --filter !test-fails run test -- --allowOnly", | |||
"test:ci": "cross-env CI=true pnpm -r --stream --filter !@vitest/monorepo --filter !test-fails --filter !test-solid run test -- --allowOnly", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we exclude solid here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solid-start
does not work with node 14 because it uses stream/web
which is implemented node 16+.
https://github.com/solidjs/solid-start/blob/9ef4c5bff3aef765251f64268d7cf767d2acfd46/packages/start/runtime/devServer.js#L2
Should I write some scripts to exclude it only from node 14?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But how is the tests passing on main branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handler I added in this PR catches the error.
On the main branch, it is not failing because it is unhandled promise rejection.
examples/solid test: failed to load config from /home/runner/work/vitest/vitest/examples/solid/vite.config.mjs
examples/solid test: (node:2602) UnhandledPromiseRejectionWarning: Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'stream' imported from /home/runner/work/vitest/vitest/node_modules/.pnpm/solid-start@0.1.0-alpha.61_solid-js@1.3.8+vite@2.8.4/node_modules/solid-start/runtime/devServer.js
examples/solid test: at new NodeError (internal/errors.js:322:7)
examples/solid test: at packageResolve (internal/modules/esm/resolve.js:732:9)
examples/solid test: at moduleResolve (internal/modules/esm/resolve.js:773:18)
examples/solid test: at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:887:11)
examples/solid test: at Loader.resolve (internal/modules/esm/loader.js:89:40)
examples/solid test: at Loader.getModuleJob (internal/modules/esm/loader.js:242:28)
examples/solid test: at ModuleWrap.<anonymous> (internal/modules/esm/module_job.js:76:40)
examples/solid test: at link (internal/modules/esm/module_job.js:75:36)
examples/solid test: (Use `node --trace-warnings ...` to show where the warning was created)
examples/solid test: (node:2602) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
examples/solid test: (node:2602) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
The run below includes the change in this PR. It catched the error and it failed correctly.
https://github.com/vitest-dev/vitest/runs/5364311919?check_suite_focus=true
examples/solid test: failed to load config from /home/runner/work/vitest/vitest/examples/solid/vite.config.mjs
examples/solid test: ⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯
examples/solid test: Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'stream' imported from /home/runner/work/vitest/vitest/node_modules/.pnpm/solid-start@0.1.0-alpha.61_solid-js@1.3.8+vite@2.8.4/node_modules/solid-start/runtime/devServer.js
examples/solid test: at new NodeError (internal/errors.js:322:7)
examples/solid test: at packageResolve (internal/modules/esm/resolve.js:732:9)
examples/solid test: at moduleResolve (internal/modules/esm/resolve.js:773:18)
examples/solid test: at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:887:11)
examples/solid test: at Loader.resolve (internal/modules/esm/loader.js:89:40)
examples/solid test: at Loader.getModuleJob (internal/modules/esm/loader.js:242:28)
examples/solid test: at ModuleWrap.<anonymous> (internal/modules/esm/module_job.js:76:40)
examples/solid test: at link (internal/modules/esm/module_job.js:75:36) {
examples/solid test: code: 'ERR_MODULE_NOT_FOUND'
examples/solid test: }
examples/solid test: Failed
`solid-start` does not work with node 14 because it uses `stream/web` which is implemented node 16+. https://github.com/solidjs/solid-start/blob/9ef4c5bff3aef765251f64268d7cf767d2acfd46/packages/start/runtime/devServer.js#L2
Sometime it fails with timeout exceeded
73797ef
to
de1038b
Compare
This PR adds a unhandled error handling which can catch more cases than this one.
vitest/packages/vitest/src/node/cli-api.ts
Lines 70 to 75 in 4858541
refs #873
solid-start
does not work with node 14 because it usesstream/web
which is implemented node 16+.https://github.com/solidjs/solid-start/blob/9ef4c5bff3aef765251f64268d7cf767d2acfd46/packages/start/runtime/devServer.js#L2
I have excluded from
pnpm test:ci
. Should I write some scripts to exclude it only from node 14?