fix: Discover protocol should only parse stdout - #22903
Conversation
Currently JsonLinesParser::from_line is called for both stdout and stderr, so trait implementers cannot distinguish stdout and stderr. Define a separate JsonLinesParser::from_stderr_line to make the stdout/stderr distinction explicit, and update use sites. This is not a behaviour change. AI disclosure: Partially written by Codex and GPT-5.5.
rustbot
added
the
S-waiting-on-review
|
cc @davidbarsky I'm pretty sure the old behaviour was entirely accidental due to the old trait API, but let me know what you think. |
| } | ||
|
|
||
| fn from_stderr_line(&self, line: &str, _error: &mut String) -> Option<DiscoverProjectMessage> { | ||
| tracing::warn!(%line, "discover command stderr"); |
There was a problem hiding this comment.
I'm on the edge about this. Is every stderr line really a warning? I see three options:
- Do nothing, leave it as
warn!(). I don't like this. - Downgrade to
info!()or evendebug!(). - Expect JSON with only the level and the message.
There was a problem hiding this comment.
stderr output is definitely rare, or we'd have hit this issue sooner! :)
I picked warn! because I thought it was nice to see the stderr output by default. That said, info! also seems fine: it solves my issue with unexpected logging from the rust-project binary breaking people's rust-analyzer setup.
Switched to info! and amended the commit.
ed5af79 to
ae9fa8c
Compare
I think the intention was have all errors go over JSON, but unlike Chayim, I don't think I have concerns about using (side note: I got some rust-project changes, if you got a moment: facebook/buck2#1398) |
|
You need to |
Previously we read both stdout and stderr in the discover protocol. Depending on the tool generating rust-project JSON, this meant that a single stderr log message could break discovery. Instead, only look for JSON from the discover command's stdout, and forward stderr to the rust-analyzer logs. Update both the implementation and the discover protocol docs to reflect this behaviour. AI disclosure: Code partly written by GPT-5.5.
ae9fa8c to
3bbccf6
Compare
|
Oops, fixed. |
rustbot
removed
the
S-waiting-on-review
5 participants
Currently r-a expects JSON on both stdout and stderr from the discover command. This means that the discover command can't log any warnings, and any accidental stderr logs breaks discovery entirely.
Instead, only parse stdout, and update
JsonLinesParserto require implementors to be explicit.