Skip to content

internal: Log flycheck errors #17033

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

Merged
merged 1 commit into from
Apr 11, 2024
Merged

internal: Log flycheck errors #17033

merged 1 commit into from
Apr 11, 2024

Conversation

ShoyuVanilla
Copy link
Member

Resolves #16969

The non-cargo messages are appended to the error strings here;

fn from_line(line: &str, error: &mut String) -> Option<Self> {
let mut deserializer = serde_json::Deserializer::from_str(line);
deserializer.disable_recursion_limit();
if let Ok(message) = JsonMessage::deserialize(&mut deserializer) {
return match message {
// Skip certain kinds of messages to only spend time on what's useful
JsonMessage::Cargo(message) => match message {
cargo_metadata::Message::CompilerArtifact(artifact) if !artifact.fresh => {
Some(CargoCheckMessage::CompilerArtifact(artifact))
}
cargo_metadata::Message::CompilerMessage(msg) => {
Some(CargoCheckMessage::Diagnostic(msg.message))
}
_ => None,
},
JsonMessage::Rustc(message) => Some(CargoCheckMessage::Diagnostic(message)),
};
}
error.push_str(line);
error.push('\n');
None
}

that one is formatted into Err here;

pub(crate) fn join(mut self) -> io::Result<()> {
let _ = self.child.0.kill();
let exit_status = self.child.0.wait()?;
let (read_at_least_one_message, error) = self.thread.join()?;
if read_at_least_one_message || exit_status.success() {
Ok(())
} else {
Err(io::Error::new(io::ErrorKind::Other, format!(
"Cargo watcher failed, the command produced no valid metadata (exit code: {exit_status:?}):\n{error}"
)))
}
}

and finally, this PR appends it at the end of existing Flycheck error message

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 8, 2024
@lnicola
Copy link
Member

lnicola commented Apr 10, 2024

r? @davidbarsky

Not sure if it matters, but we can probably do something like:

tracing::error!(%error, formatted_handle, "flycheck failed to run");

@davidbarsky
Copy link
Contributor

r? @davidbarsky

Not sure if it matters, but we can probably do something like:

tracing::error!(%error, formatted_handle, "flycheck failed to run");

Yeah, this PR looks good to me (I don't think bors accepts commands from me? I've never tried), but reformat the expression like that is a nice bonus!

(It'd be neat if I could figure out how write that transform with SSR.)

@Veykril
Copy link
Member

Veykril commented Apr 11, 2024

I don't think bors accepts commands from me? I've never tried)

It doesn't (at least not yet), but feel free to approve PRs via github nevertheless!

(It'd be neat if I could figure out how write that transform with SSR.)

I don't think SSR can interpolate from strings (changing the {} captures)

@Veykril
Copy link
Member

Veykril commented Apr 11, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Apr 11, 2024

📌 Commit f7a66fd has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 11, 2024

⌛ Testing commit f7a66fd with merge 657b33b...

@bors
Copy link
Contributor

bors commented Apr 11, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 657b33b to master...

@bors bors merged commit 657b33b into rust-lang:master Apr 11, 2024
@lnicola lnicola changed the title Log flycheck errors internal: Log flycheck errors Apr 12, 2024
@ShoyuVanilla ShoyuVanilla deleted the flyck-log branch July 12, 2024 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flycheck error message not logged
6 participants