Ignore nrepl process stderr #50
Conversation
| @@ -46,13 +46,19 @@ const start = (): Promise<CljConnectionInformation> => { | |||
| }); | |||
|
|
|||
| nreplProcess.stderr.on('data', data => { | |||
| console.info('nrepl stderr =>', data.toString()); | |||
There was a problem hiding this comment.
Looks good. Not blocking for now, but just a thought: we could wire this up to a vscode.OutputChannel, so users can see any output without having to run the extension under debugging mode (I think console.info only shows up when debugging the extension?)
But probably not worth doing in this review, probably needs a bit more thought.
There was a problem hiding this comment.
@mhansen Good idea. I'm gonna merge the PR as is but will create the related issue soon.
There was a problem hiding this comment.
Alright that's a good thing we should be thinking. We'll discuss it on #52.
| stop(); | ||
| return reject(`This error happened with our nREPL process: ${data}`); | ||
| return reject(); |
There was a problem hiding this comment.
Just checking: Do we want to reject() even if the exit code is 0? I suppose we do, if it exits before we manage to resolve() the Promise by getting the connection info, we should probably treat that as an error. What do you think?
| stop(); | ||
| return reject(`This error happened with our nREPL process: ${data}`); | ||
| return reject(); |
There was a problem hiding this comment.
We could reject the promise with an error string, then it'd show up in the vscode.window.showErrorMessage. This would make it easier both for users to understand what's going on, and for us to understand what's going wrong with bug reports in the future. Perhaps we could say here something like 'nREPL unexpectedly exited, error code ${code}'.
There was a problem hiding this comment.
@mhansen Again, good idea. I'll add the related issue.
| stop(); | ||
| return reject(`Our nREPL was closed. Code: ${code} / Signal: ${signal}`); | ||
| return reject(); |
There was a problem hiding this comment.
As above, consider adding an error message, to help us (& the user) debug what's going on?
nrepl process uses stderr to notify stuff other than error. When it's fired, it doesn't mean that nrepl didn't work.
This closes #47 and closes #48.