Ignore errors parsing slicec JSON output

[pepone]

Fix #4043

[externl]

Is this variable still needed?

[externl]

Should we first check that the line (whitespace stripped) begins and ends with { and } instead of just ignoring all Json exceptions. Otherwise it might not be obvious if we break something?

[InsertCreativityHere]

I think I agree, blindly eating all JsonExceptions feels a little dangerous to me...

[InsertCreativityHere]

I think it would be better to just have a check at the top here, where if the first character (or maybe non-whitespace character?) isn't {, just immediately return, instead of invoking the JSONParser.

We could still keep the exception handling centralized in the parser though, except instead of silently eating exceptions, it could Log.LogError(...)

[pepone]

Should we first check that the line (whitespace stripped) begins and ends with { and } instead of just ignoring all Json exceptions. Otherwise it might not be obvious if we break something?

The plug-in should not code defensively against possible bugs in the compiler.

I think a better compromise here would be to emit all messages as JSON when --diagnostic JSON is set, then log any exceptions as fatal errors instead of eating them.

The compiler can emit {summary: "Compilation failed with 1 error(s)" status: 1} instead of Failed: Compilation failed with 1 error(s)

[InsertCreativityHere]

The plug-in should not code defensively against possible bugs in the compiler.

Agreed. but no matter what we do, the compiler will sometimes emit non-json stuff, and I don't think we should eat and ignore them. For example, if you pass a bad CLI option, the compiler exits before it even has a chance to properly set json-diagnostics.

I think a better compromise here would be to emit all messages as JSON

Actually, could we go even simpler, and just not emit this summary message at all?
JSON mode is basically only meant for tools/plug-ins. And if they're just going to ignore the message, why emit it?
All the information in the message is recoverable anyways. You already know how many errors there were, and if that number is greater than 0, then the status code is going to be 1.

[pepone]

Actually, could we go even simpler, and just not emit this summary message at all?

Yes tools are just going to ignore it.

Agreed. but no matter what we do, the compiler will sometimes emit non-json stuff,

Right, the ignoring of JSON was just to work around the summary message not being JSON formatted.

[InsertCreativityHere]

Cool, I'll open a PR for slicec to suppress the summary message when using JSON mode!
Just give me a couple minutes.

[InsertCreativityHere]

See: icerpc/slicec#702

[InsertCreativityHere]

With the changes made to the Slice compiler, the original issue should be fixed now.

But I still think that this centralized parser is nicer, and worth doing. The only change I would recommend is emitting an error message when we hit an exception in the JSON parsing, instead of ignoring them.