Omnisharp flame is green even though the omnisharp server errored

[akshita31]

Steps to reproduce:

Delete the Omnisharp executable(ensure that the install.Lock is present so that a new download is not triggered) and then restart the server.

Expected:
OmniSharp flame in the status bar observer should turn red.

Actual:
Omnisharp flame is green, which indicates that the omnisharp server is running.

Diagnosis:

A simple tweak here is to add another event to the OmnisharpStatusBarObserver, and the PR for that is here : #2349. However this makes me think that should we have a "success" message returned from omnisharp once it has successfully spawned ( nodejs/help#1191)., and wait for that message before we say that 'OmniSharp server has started'. Doing this might also help remove the flakiness that we have in the integration tests.

When we fire the event here : https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/omnisharp/server.ts#L329, we are not sure that the server has started because upto this point we haven't seen any such event that suggests so.

Thoughts @rchande @DustinCampbell @TheRealPiotrP @colombod

[rchande]

I thought we got rid of/are getting rid of install.lock? Seems like doing that could make this scenario less confusing because users don't have to understand what that file is and when to remove it.

[akshita31]

@rchande Yes that is a thing I will take up, but the issue I am trying to convey here is more than that.

Assuming that there is an std error here : https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/omnisharp/server.ts#L511, we are not catching this error as it will be produced asynchronously and we fire the "server started" event here : https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/omnisharp/server.ts#L329, even before we actually know that omnisharp has started successfully.
We await the omnisharp running status in our integration test and my guess is that the flakiness there is also because of this reason.

If we can return an event from OmniSharp saying - "Hey, I have started" and then fire the server started event, I think that will solve a number of problems. Did that make sense ?

[rchande]

@akshita31 Sure, makes sense. Do we really have to add a dedicated event to o# though? I bet we could just listen for any command line output from O# and consider that to mean that it started.

[akshita31]

@rchande Yes, listening to any data on the standard output of the process should be enough. I'll look into how to do this further.