Addon Vitest: Improve error message in testing widget modal #33481
Conversation
|
View your CI Pipeline Execution ↗ for commit 28405e0
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @code/addons/vitest/src/utils.ts:
- Line 8: The stack field currently concatenates error.message with error.stack
using optional chaining which yields "message undefined" when error.stack is
missing; update the expression used for the stack property so it checks
error.stack first and, if present, returns error.message + '\n' +
error.stack.replace(error.message, '') (or similar newline-separated
formatting), otherwise just return error.message; locate the object building the
stack property (the line with stack: error.message + ' ' +
error.stack?.replace(error.message, '')) and replace it with a conditional that
avoids referencing undefined error.stack.
- Line 7: Update the misleading comment above the stack-manipulation logic in
utils.ts: replace "avoid duplicating the error message in the stack trace" with
a comment that reflects the actual behavior such as "ensure the error message is
included in the stack trace" so the comment accurately describes that the code
prepends the error message to the stack.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/vitest/src/utils.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/vitest/src/utils.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/vitest/src/utils.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/vitest/src/utils.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/utils.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/vitest/src/utils.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
- GitHub Check: nx
| return { | ||
| message: error.message, | ||
| name: error.name, | ||
| // avoid duplicating the error message in the stack trace |
There was a problem hiding this comment.
Update the misleading comment.
The comment states "avoid duplicating the error message" but the code on line 8 now explicitly prepends the error message to the stack. Update the comment to reflect the actual intent, such as "ensure the error message is included in the stack trace."
📝 Proposed fix
- // avoid duplicating the error message in the stack trace
+ // ensure the error message is included in the stack trace
stack: error.message + ' ' + error.stack?.replace(error.message, ''),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // avoid duplicating the error message in the stack trace | |
| // ensure the error message is included in the stack trace | |
| stack: error.message + ' ' + error.stack?.replace(error.message, ''), |
🤖 Prompt for AI Agents
In @code/addons/vitest/src/utils.ts at line 7, Update the misleading comment
above the stack-manipulation logic in utils.ts: replace "avoid duplicating the
error message in the stack trace" with a comment that reflects the actual
behavior such as "ensure the error message is included in the stack trace" so
the comment accurately describes that the code prepends the error message to the
stack.
| name: error.name, | ||
| // avoid duplicating the error message in the stack trace | ||
| stack: error.stack?.replace(error.message, ''), | ||
| stack: error.message + ' ' + error.stack?.replace(error.message, ''), |
There was a problem hiding this comment.
Handle undefined error.stack to prevent malformed output.
If error.stack is undefined, the optional chaining returns undefined, and the string concatenation produces "message undefined" (e.g., "Something went wrong undefined"), which is incorrect.
🐛 Proposed fix
- stack: error.message + ' ' + error.stack?.replace(error.message, ''),
+ stack: error.stack ? `${error.message}\n${error.stack.replace(error.message, '')}` : error.message,This ensures:
- If
error.stackexists, the message is prepended with a newline separator (conventional format). - If
error.stackis undefined, the stack field contains just the error message.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| stack: error.message + ' ' + error.stack?.replace(error.message, ''), | |
| stack: error.stack ? `${error.message}\n${error.stack.replace(error.message, '')}` : error.message, |
🤖 Prompt for AI Agents
In @code/addons/vitest/src/utils.ts at line 8, The stack field currently
concatenates error.message with error.stack using optional chaining which yields
"message undefined" when error.stack is missing; update the expression used for
the stack property so it checks error.stack first and, if present, returns
error.message + '\n' + error.stack.replace(error.message, '') (or similar
newline-separated formatting), otherwise just return error.message; locate the
object building the stack property (the line with stack: error.message + ' ' +
error.stack?.replace(error.message, '')) and replace it with a conditional that
avoids referencing undefined error.stack.
Closes #
What I did
Small but important fix, sometimes when running tests through the testing widget, the error message was not included in the error stack and on
FATAL_ERRORwe would end up missing that important bit of information before logging to the user. This PR fixes that.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.