From 8ffbe86dfd24ffcc11ec37bceca9102260d59db2 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 29 Jan 2020 13:40:27 +0100 Subject: Parse cargo output a line at a time. We previously used serde's stream deserializer to read json blobs from the cargo output. It has an issue though: If the deserializer encounters invalid input, it gets stuck reporting the same error again and again because it is unable to foward over the input until it reaches a new valid object. Reading a line at a time and manually deserializing fixes this issue, because cargo makes sure to only outpu one json blob per line, so should we encounter invalid input, we can just skip a line and continue. The main reason this would happen is stray printf-debugging in procedural macros, so we still report that an error occured, but we handle it gracefully now. Fixes #2935 --- crates/ra_cargo_watch/Cargo.toml | 1 + crates/ra_cargo_watch/src/lib.rs | 26 +++++++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/crates/ra_cargo_watch/Cargo.toml b/crates/ra_cargo_watch/Cargo.toml index 49e06e0d3..dd814fc9d 100644 --- a/crates/ra_cargo_watch/Cargo.toml +++ b/crates/ra_cargo_watch/Cargo.toml @@ -11,6 +11,7 @@ log = "0.4.3" cargo_metadata = "0.9.1" jod-thread = "0.1.0" parking_lot = "0.10.0" +serde_json = "1.0.45" [dev-dependencies] insta = "0.13.0" diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index 9af9c347d..e015692fa 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -9,7 +9,7 @@ use lsp_types::{ }; use std::{ collections::HashMap, - io::BufReader, + io::{BufRead, BufReader}, path::PathBuf, process::{Command, Stdio}, sync::Arc, @@ -350,13 +350,29 @@ impl WatchThread { // which will break out of the loop, and continue the shutdown let _ = message_send.send(CheckEvent::Begin); - for message in - cargo_metadata::parse_messages(BufReader::new(command.stdout.take().unwrap())) - { + // We manually read a line at a time, instead of using serde's + // stream deserializers, because the deserializer cannot recover + // from an error, resulting in it getting stuck, because we try to + // be resillient against failures. + // + // Because cargo only outputs one JSON object per line, we can + // simply skip a line if it doesn't parse, which just ignores any + // erroneus output. + let stdout = BufReader::new(command.stdout.take().unwrap()); + for line in stdout.lines() { + let line = match line { + Ok(line) => line, + Err(err) => { + log::error!("Couldn't read line from cargo: {:?}", err); + continue; + } + }; + + let message = serde_json::from_str::(&line); let message = match message { Ok(message) => message, Err(err) => { - log::error!("Invalid json from cargo check, ignoring: {}", err); + log::error!("Invalid json from cargo check, ignoring ({}): {} ", err, line); continue; } }; -- cgit v1.2.3 From 4ec5f6e25850b3064b258739eabefdeb8a4bd1b5 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 29 Jan 2020 13:51:20 +0100 Subject: Change error output to make a bit more sense --- crates/ra_cargo_watch/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index e015692fa..ea7ddc86b 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -363,7 +363,7 @@ impl WatchThread { let line = match line { Ok(line) => line, Err(err) => { - log::error!("Couldn't read line from cargo: {:?}", err); + log::error!("Couldn't read line from cargo: {}", err); continue; } }; @@ -372,7 +372,11 @@ impl WatchThread { let message = match message { Ok(message) => message, Err(err) => { - log::error!("Invalid json from cargo check, ignoring ({}): {} ", err, line); + log::error!( + "Invalid json from cargo check, ignoring ({}): {:?} ", + err, + line + ); continue; } }; -- cgit v1.2.3