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

Slice.Tools JSON error #4043

Closed
bernardnormier opened this issue Sep 9, 2024 · 8 comments · Fixed by icerpc/slicec#702
Closed

Slice.Tools JSON error #4043

bernardnormier opened this issue Sep 9, 2024 · 8 comments · Fixed by icerpc/slicec#702
Assignees
Labels
bug Something isn't working
Milestone

Comments

@bernardnormier
Copy link
Member

I am getting this error whenever I get a redefinition error in a Slice file:

    /Users/bernard/builds/icerpc-csharp/tools/IceRpc.Slice.Tools/IceRpc.Slice.Tools.targets(60,5): error : Error parsing JSON: 'F' is an invalid start of a value. LineNumber: 0 | BytePositionInLine: 0.. JSON content: Failed: Compilation failed with 3 error(s)
@bernardnormier bernardnormier added the bug Something isn't working label Sep 9, 2024
@bernardnormier bernardnormier added this to the 0.4.0 milestone Sep 9, 2024
@pepone
Copy link
Member

pepone commented Sep 9, 2024

This is due to the compiler emitting non JSON diagnostics:

{"message":"redefinition of 'Locator'","severity":"error","span":{"start":{"row":20,"col":11},"end":{"row":20,"col":18},"file":"slice\\Ice\\Locator.slice"},"notes":[{"message":"'Locator' was previously defined here","span":{"start":{"row":14,"col":11},"end":{"row":14,"col":18},"file":"slice\\Ice\\Locator.slice"}}],"error_code":"E012"}
Failed: Compilation failed with 1 error(s)

@externl
Copy link
Member

externl commented Sep 9, 2024

It used to be (I think) that JSON output was emitted to stdout and the filed summary was sent to stderr. IIRC rust does something similar. I thought that we used to only read from stdout, or at least not emit errors if JSON unmarshaling failed.

@pepone
Copy link
Member

pepone commented Sep 9, 2024

No quite correct.

We used to only emit JSON to STDERR for error messages. But now with --telemetry we also emit telemetry info as JSON formatted data to STDOUT.

I guess the simple is to ignore this non JSON messages from STDOUT as we did before, and just collect the STDOUT JSON messages that has the expected telemetry data.

@InsertCreativityHere
Copy link
Member

that JSON output was emitted to stdout and the filed summary was sent to stderr

It's the other way around with slicec, the Compilation failed with ... message is sent to stdout,
and all the errors are sent to stderr (either as JSON or text).

@InsertCreativityHere
Copy link
Member

InsertCreativityHere commented Sep 9, 2024

Another, maybe easier fix is to change slicec-cs to emit it's telemetry to stderr instead.
That way stderr is purely JSON, and stdout is purely text.

And this would be a one-line change in slicec-cs to implement. Just change this:

println!(
r#"{{ "hash": "{hash}", "contains_slice1": {contains_slice1}, "contains_slice2": {contains_slice2}, "src_file_count": {src_file_count}, "ref_file_count": {ref_file_count} }}"#
);
To eprintln instead.

@pepone
Copy link
Member

pepone commented Sep 9, 2024

Another, maybe easier fix is to change slicec-cs to emit it's telemetry to stderr instead.

Seems wrong to emit non error data to stderr

@externl
Copy link
Member

externl commented Sep 9, 2024

Yeah, I'd rather just ignore non-JSON output in the tools.

@InsertCreativityHere
Copy link
Member

Fair enough, I agree it'd be weirder, was just pitching ideas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants