feat(cli): show backtrace on panic if available#8647
Conversation
🦋 Changeset detectedLatest commit: a16b961 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughIntroduces a dedicated panic handler flow: a new Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
|
@ematipico Added in biomejs/website#3770, I am not sure if this is the right place to add though |
|
Yes it's correct :) |
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
Oh good, I was wishing we had this recently!
| @@ -0,0 +1,9 @@ | |||
| --- | |||
| "@biomejs/biome": minor | |||
There was a problem hiding this comment.
Any chance this could be a patch instead of a minor? It's technically a new feature, but I would love to have this available on main.
There was a problem hiding this comment.
We could add the code in main, and the changeset + docs in next
There was a problem hiding this comment.
To be honest I would still target the next branch to keep it simple. Let's just ship the next minor if we want this (and other new features) soon.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_cli/src/panic.rs (2)
14-24: Well-structured panic handler flow.The refactoring cleanly separates message construction from output. Writing to stderr before tracing is defensive—if tracing panics, users still see the error.
Minor suggestion: Consider a more descriptive expect message on line 15, e.g.,
"Failed to format panic message", though in practiceStringwrites cannot fail except under extreme memory pressure.
26-79: Consider adding documentation.While not critical, rustdoc comments on
write_errorwould improve maintainability—describing its purpose (formats panic information with optional backtrace) and return value (formatted string or fmt::Error).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_cli/src/panic.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_cli/src/panic.rs
🧠 Learnings (6)
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_cli/src/panic.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement the `diagnostic` function to provide error messages explaining what the error is, why it is triggered, and what the user should do
Applied to files:
crates/biome_cli/src/panic.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Implement error recovery in list parsing using `or_recover()` to wrap unparseable tokens in a `BOGUS_*` node and consume tokens until a recovery token is found
Applied to files:
crates/biome_cli/src/panic.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ParseSeparatedList` and `ParseNodeList` for parsing lists with error recovery to avoid infinite loops
Applied to files:
crates/biome_cli/src/panic.rs
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: Applies to **/*.rs : Use the `dbg!()` macro for debugging output in Rust tests and code
Applied to files:
crates/biome_cli/src/panic.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the `dbg_write!` macro to debug formatter output instead of other logging methods
Applied to files:
crates/biome_cli/src/panic.rs
🧬 Code graph analysis (1)
crates/biome_cli/src/panic.rs (1)
crates/biome_diagnostics/src/display/backtrace.rs (1)
capture(29-33)
⏰ 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). (8)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Documentation
- GitHub Check: autofix
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
🔇 Additional comments (3)
crates/biome_cli/src/panic.rs (3)
1-6: LGTM!The backtrace imports are correctly added from
std::backtraceand align with the PR's goal to expose stack traces whenRUST_BACKTRACE=1is enabled.
26-59: Excellent refactoring with consistent error handling.Extracting
write_errorimproves testability and maintainability. The consistent use of?for error propagation is more idiomatic than the.expect()calls suggested in past review comments—since the caller already handles theResult, this approach is cleaner.
61-76: Solid backtrace implementation with good user guidance.The conditional backtrace handling is well-structured:
- Shows the trace when captured
- Provides actionable guidance when disabled
- Handles unsupported platforms gracefully
The unconditional
Backtrace::capture()call is appropriate—when backtraces are disabled, it returns immediately without overhead.
Hey, I'm working on a new version that should make more stuff possible. Do you have a wish list? |
4 participants
Summary
Added the backtrace to the panic report if it's enabled and captured successfully (
RUST_BACKTRACE=1needs to be set on running).Example
Test Plan
Manually tested
Docs
biomejs/website#3770