Restarting the server breaks flycheck for open buffers

[razzmatazz]

I have noticed that each time I restart the server manually all the .cs buffers have their flycheck in "defunct mode", i.e. I need to re-open the file to get flycheck check that buffer again.

[razzmatazz]

The problem appears to be because we're breaking flycheck contract in :start handler for flycheck-define-generic-checker which is omnisharp--flycheck-start on our side.

Flycheck documentation on flycheck-define-generic-checker is explicit on this by saying:

• "A syntax checker must call CALLBACK at least once with a STATUS that finishes the current syntax checker. Otherwise Flycheck gets stuck at the current syntax check with this syntax checker."
• see https://github.com/flycheck/flycheck/blob/master/flycheck.el#L1526

I think there are two sequences that can trigger the problem:

• the server does not respond, like ever, for a given request-id that does codecheck on the server (for some reason);
• there is a race or an error, or something else and omnisharp--send-command-to-server fails to invoke response-handler

To fix this we probably need to:

• add a timeout for receiving messages from the server and then invoking response-handler with callback special error value (or add new param error-handler and invoke that)
• update omnisharp--flycheck-start to handle erroneous condition/failure

[mikavilpas]

I like the idea of having a timeout callback. Or would it be a generic error callback?

[razzmatazz]

I would sugest a general callback, because it could as well be "server-error" or "server-disconnected/terminated" or something else, not just a timeout.. What do you think? Maybe you have a better idea..

I didn't read the code that much yet to be able to tell if passing nil or some kind of error value to response-handler would break more things than fix :), rather than adding a new &optional callback parameter to omnisharp--send-command-to-server..

[mikavilpas]

I think a general error handler would be the best. Maybe also the simplest to implement. I think the server doesn't classify its responses as successful or failed ones, so we'd have one for a timeout, and another one for an exception of some kind. Can you think of something else?

On Apr 4, 2017 11:15, "Saulius Menkevičius" notifications@github.com wrote:

I would sugest a general callback, because well it could as well be "server-error" or "server-disconnected/terminated" or something else, not only a timeout.. What do you think? Maybe you have a better idea..

I didn't read the code that much yet to be able to tell if passing nil or some kind of error value to respone-handler would break more things than fix :), rather than adding a new parameter to omnisharp--send-command-to- server..

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OmniSharp/omnisharp-emacs/issues/283#issuecomment-291427309, or mute the thread https://github.com/notifications/unsubscribe-auth/AASW978J-fpOwEbYx0GvBEfKxs4vwWruks5rsfw7gaJpZM4MTCFt .

[razzmatazz]

Do you mean, a handler that is set globally, not via omnisharp--send-command-to-server? But then we wouldn't be able to invoke flycheck result handler of a particular failed request from a global handler.

[razzmatazz]

Yes, this is probably more like 'crash'/'connection error' handler rather than "error" response from a working server.

[razzmatazz]

Removing from 4.2, doesn't happen at least for me that often.