Skip to content
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

--message-format=json doesn't emit every error message #5992

Closed
koute opened this issue Sep 7, 2018 · 8 comments · Fixed by #6081
Closed

--message-format=json doesn't emit every error message #5992

koute opened this issue Sep 7, 2018 · 8 comments · Fixed by #6081

Comments

@koute
Copy link
Member

koute commented Sep 7, 2018

Steps to reproduce:

$ cargo new foo
$ cd foo
$ echo 'yew = "= 0.4.0"' >> Cargo.toml

Replace src/main.rs with this:

#[macro_use]
extern crate yew;

fn main() {
    html! {
        <div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
            <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
        </div>
    }
}

Then if you build it:

$ cargo build --target=wasm32-unknown-unknown

you'll get the following error:

error: recursion limit reached while expanding the macro `stringify`
  --> src/main.rs:5:5
   |
5  | /     html! {
6  | |         <div>
7  | |             <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
8  | |             <div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div>
...  |
17 | |         </div>
18 | |     }
   | |_____^
   |
   = help: consider adding a `#![recursion_limit="128"]` attribute to your crate
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error: aborting due to previous error

error: Could not compile `foo`.

however if you build it with:

cargo build --target=wasm32-unknown-unknown --message-format=json

then the error message is missing from the output. I'd expect cargo to emit a line with {"reason": "compiler-message", ...} just as it usually does for errors; instead it emits no "compiler-message"s, e.g. the output of this (to match only "compiler-message"s):

$ cargo build --target=wasm32-unknown-unknown --message-format=json | ruby -ne 'require "json"; require "pp"; j = JSON.parse( $_ ); if j["reason"] == "compiler-message"; pp j; end'

is empty.

@ehuss
Copy link
Contributor

ehuss commented Sep 7, 2018

It looks like serde_json chokes on the ~1MB json structure with a recursion error (it includes a very deeply nested macro expansion). Cargo only displays that error if the compile succeeds, so it gets suppressed. Unfortunately it looks like serde_json is hardcoded to a specific recursion depth of 128 which is not enough in this case. I'm uncertain of the best way to address this.

@alexcrichton
Copy link
Member

@ehuss hm I'm not quite following how the recursion depth and serde_json come into play here. Could you point the part of Cargo that's causing this problem? (or is this in rustc?)

@ehuss
Copy link
Contributor

ehuss commented Sep 11, 2018

It's a little confusing because there are two recursion errors. There's the recursion error that happens inside rustc during macro expansion which is the error we're trying to report. However, rustc includes a giant JSON structure with all the expansions nested to the point where it failed.

When the JSON is parsed by cargo (here), it fails with a RecursionLimitExceeded error from serde_json. serde_json's limit is hardcoded to 128 here.

When the json parsing fails, the error gets dropped here if the process also fails.

The few solutions I've thought of aren't very good. We could change serde_json to allow us to set the recursion limit, but to what value? See also serde-rs/json#162. rustc can emit arbitrarily deep structures (unless the stack overflows).

Cargo could emit a warning with the json string when it fails to parse the JSON, so you can at least see that something went wrong.

@alexcrichton
Copy link
Member

Ah ok, thanks for the explanation, that definitely makes sense!

I wonder if we could perhaps tack on more error information to the process failure as well, to be emitted as warnings maybe? Emitting a warning otherwise sounds fine by me

@dtolnay
Copy link
Member

dtolnay commented Sep 13, 2018

I made a JSON library that does not use any recursion. Maybe that would be useful here?
https://github.com/dtolnay/miniserde

@kjeremy
Copy link

kjeremy commented Sep 17, 2018

@dtolnay I saw your announcement on reddit and immediately thought of this issue.

@dwijnand
Copy link
Member

Just for the record here, there was an attempt in #6032 switching to a library called miniserde, but that change wasn't accepted because it introduced a second json parser and serialization implementation.

bors added a commit that referenced this issue Sep 24, 2018
Fix missing messages when --message-format=json is deeply nested

This commit switches from `serde_json::Value` to [`RawValue`](https://docs.rs/serde_json/1.0/serde_json/value/struct.RawValue.html), which can process arbitrarily deeply nested JSON content without recursion.

Fixes #5992. @ehuss
bors added a commit that referenced this issue Nov 9, 2018
Show error on JSON parse error and nonzero exit.

If JSON parsing failed, and rustc exits nonzero, the error was lost.
This would have made it easier to diagnose #5992.
@SichangHe
Copy link

For future references, disable_recursion_limit would be one way to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants