fix(jest-circus): prevent crash when asyncError is undefined for non-Error throws#16003
fix(jest-circus): prevent crash when asyncError is undefined for non-Error throws#16003ashoknarayan wants to merge 14 commits intojestjs:mainfrom
Conversation
|
|
✅ Deploy Preview for jestjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
babel-jest
babel-plugin-jest-hoist
babel-preset-jest
create-jest
@jest/diff-sequences
expect
@jest/expect-utils
jest
jest-changed-files
jest-circus
jest-cli
jest-config
@jest/console
@jest/core
@jest/create-cache-key-function
jest-diff
jest-docblock
jest-each
@jest/environment
jest-environment-jsdom
@jest/environment-jsdom-abstract
jest-environment-node
@jest/expect
@jest/fake-timers
@jest/get-type
@jest/globals
jest-haste-map
jest-jasmine2
jest-leak-detector
jest-matcher-utils
jest-message-util
jest-mock
@jest/pattern
jest-phabricator
jest-regex-util
@jest/reporters
jest-resolve
jest-resolve-dependencies
jest-runner
jest-runtime
@jest/schemas
jest-snapshot
@jest/snapshot-utils
@jest/source-map
@jest/test-result
@jest/test-sequencer
@jest/transform
@jest/types
jest-util
jest-validate
jest-watcher
jest-worker
pretty-format
commit: |
|
Hi @alex-all3dp, I’ve implemented the fixes described for this issue. However, I’m running into some CI failures related to the new test I added and I’m not fully sure what’s going wrong there. Could you please take a look and let me know if I’m missing something in how the test should be structured? I tried to follow the existing patterns, but I might have overlooked something. Thanks! |
|
Hi @ashoknarayan, thanks for picking this up! The logic changes in formatNodeAssertErrors.ts and utils.ts look correct. The CI failures are all coming from the new test file. The import '../mocks/testUtils' resolves to a path that doesn't exist in the package, which causes a module-not-found error that takes down the whole shard (which is why unrelated tests in shard 2 are failing too). There are test helpers in jest-circus/src/tests/ that show how to run jest-in-jest style tests, those should point you in the right direction. Worth noting is that a synchronous throw { status: 403 } won't actually hit the crash path you're fixing. The asyncError is only undefined when the error comes in through an async channel like a done callback or an unhandled subscription, because that's the case where jest-circus never gets to capture the async stack. To cover the fix you'd need a test that replicates that pattern. One more thing from the original issue worth addressing: Circus.TestError types the second tuple element as Exception (non-optional), but at runtime it can be undefined. Fixing the type definition alongside the logic changes would be the complete fix. |
|
Hi @alex-all3dp, I’ve addressed the feedback and updated the PR.
All checks are now passing. Would appreciate it if you could take another look when you get a chance. Thanks! |
Fixes #15996
Handled cases where asyncError can be undefined when a test throws a non-Error value.
This prevents jest-circus from crashing and ensures the error is reported properly.