Print exceptions as errors #1196
Conversation
|
Tagging the issue here: #1192 |
AbhitejJohn
left a comment
There was a problem hiding this comment.
Is the error message and details the same as what the current service puts out?
| catch (Exception e) | ||
| { | ||
| onError(e.ToString()); | ||
| onOutput(e.ToString()); |
There was a problem hiding this comment.
I'm not super familiar with this code - it is weird though that we have to call both onError and onOutput for an exception?
There was a problem hiding this comment.
I agree with @AbhitejJohn point. but the exception is wired on the run result, i think we are doing the wrong thing after the runner has produced the reunResult, probably in the integration part
There was a problem hiding this comment.
look in the file doing the integration with the wasrunner :
this is where it might loose the exception as it is now serialised across
this is where results are received and passed into the kernel:
this is where the fix proposed from @AbhitejJohn will see the onError :
But I think is clear the issues is in this point:
There was a problem hiding this comment.
Just to clarify, the changes on this pr is only using onOutput(e.ToString());
onError() was deleted.
There was a problem hiding this comment.
@ocallesp : This still seems like the wrong place to make the fix since we aren't really passing the exception details through but just printing it to the output pane as Diego mentions. Can you please take a look at the suggestion above and re-work this? At the moment, this seems like a hack.
There was a problem hiding this comment.
Just to clarify, the changes on this pr is only using onOutput(e.ToString()); onError() was deleted.
No really, the point here is to fix the bug in the factory code wiring the messages from the wasmrunner and the kernel in javascript. I think we are missing the ability to complete the commands with command failed correctly. That being the case we will need better tests to make sure all is then flowing into the runresults and events that trydotnet.js consumes and then provides to the integration code with the ui part
|
@ocalles we still need the suggestion from @AbhitejJohn , the Do you have a test that shows that your fix flows into the |
Uh oh!
There was an error while loading. Please reload this page.