OutputEvent objects should respect output severity

[roblourens]

If category is stderr, then 'variables' is never called.

[isidorn]

@roblourens looking at the code we do not use severity in a case for OutputEvent with a variablesReference. It is simply ignored.

For me to investigate further I would need exact repro steps

[roblourens]

Sorry, I must have changed two things at once, because the actual problem I'm having is with this check event.body.output.length > 0. I think output shouldn't be required in this case, since it's ignored.

[roblourens]

But the fact that severity is ignored is also an issue, right? Opened a new issue for the output part - #16527

[roblourens]

I was hoping to use variablesReference for all output events, not just ones that contain objects, for consistency. If it's better to check the types that were logged and fall back on the original behavior for strings, I can do that. But I think it would be good if there could be an indicator that an object was logged as an error, or thrown. It should stand out if you're skimming the log.

[isidorn]

@roblourens but what would happen if you log an object as an error. Only the top level object would get decorated as an error or all children would also get decorated as an error.
I am not seeing a huge value in adding this. Putting it to the backlog until there is a clear use case for this.

[roblourens]

This seems really important to me. If you have an app producing lots of logs, exceptions and errors need to stand out. For node/chrome, I expect to use it for any console.error (at least with an object) or thrown exception. The first time someone misses an error because it's buried in identical logs, they will trust the console less. I'm not sure exactly what it should look like though.

[isidorn]

Ok, let me assign back to me since you think it is important.
I suggest that we only decorate the parent object in a special way and once it is expanded the children are decorated regularly

[roblourens]

@isidorn Are you still planning on this for the month? Let me know if I can help.

[isidorn]

@roblourens yeah I actually planed to look into it tomorrow. You can help me by letting me know how would you expect this to behave. Is it fine if only the top level object is colored red and then all the children as usual? That would fix your concern of making it easier to spot in a lot of output
Here's a code pointer in case you feel like tackling it https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/electron-browser/debugService.ts#L338 basically the severity should be passed and that object should have some property such that the repl viewer nicely decorates it in red

[roblourens]

Cool, yeah what you describe is just how I'd expect it to behave. I may take a look, thanks.