-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LSP server broken when using stream method #1708
Comments
thanks for looking into this! excellent deduction as well. would you like to open a PR that ensures this works for streams? you should be able to pass the options down to Logger readily since it's instantiated in since our tests and my manual testing is always with |
Thanks for providing helpful information. I might take a stab at it. |
Don't write log messages to stdout in "stream" mode. stdout is reserved for communicating with the LSP client in the "stream" mode so in that mode only write to stderr. Fixes lsp server crashing on first message it receives (or maybe LSP client closing it because it received invalid response). Also: - the callback to socket.write() was always logging the "err" parameter even if there was undefined which caused spurious lines in the console. Only actually log that if "err" is truthy. - Avoid too many newlines at the end of the log message as that also causes spurious empty lines in the log and the console. - Gracefully handle case when the response for "workspace/configuration" returns null value. Those configuration options have sane defaults and don't need to be present. - The Logger._stream property was removed as it was unused. Resolves graphql#1708
Don't write log messages to stdout in "stream" mode. stdout is reserved for communicating with the LSP client in the "stream" mode. Write all types of log messages to stderr. Fixes LSP server crashing on the first message it receives (or maybe LSP client closing it because it received an invalid response). Also: - the callback to socket.write() was always logging the "err" parameter even if there was undefined which caused spurious lines in the console. Only actually log that if "err" is truthy. - Avoid too many newlines at the end of the log message as that also causes spurious empty lines in the log and the console. - Gracefully handle the case when the response for "workspace/configuration" returns a null value. Those configuration options have sane defaults and don't need to be present. - The Logger._stream property was removed as it was unused. Resolves graphql#1708
Don't write log messages to stdout in "stream" mode. stdout is reserved for communicating with the LSP client in the "stream" mode. Write all types of log messages to stderr. Fixes LSP server crashing on the first message it receives (or maybe LSP client closing it because it received an invalid response). Also: - the callback to socket.write() was always logging the "err" parameter even if there was undefined which caused spurious lines in the console. Only actually log that if "err" is truthy. - Avoid too many newlines at the end of the log message as that also causes spurious empty lines in the log and the console. - Gracefully handle the case when the response for "workspace/configuration" returns a null value. Those configuration options have sane defaults and don't need to be present. - The Logger._stream property was removed as it was unused. Resolves graphql#1708
Don't write log messages to stdout in "stream" mode. stdout is reserved for communicating with the LSP client in the "stream" mode. Write all types of log messages to stderr. Fixes LSP server crashing on the first message it receives (or maybe LSP client closing it because it received an invalid response). Also: - the callback to socket.write() was always logging the "err" parameter even if there was undefined which caused spurious lines in the console. Only actually log that if "err" is truthy. - Avoid too many newlines at the end of the log message as that also causes spurious empty lines in the log and the console. - Gracefully handle the case when the response for "workspace/configuration" returns a null value. Those configuration options have sane defaults and don't need to be present. - The Logger._stream property was removed as it was unused. Resolves #1708
Run server with:
and send the
initialize
message to it.The server will just crash in a non-crashy way (just exit with 0 exit code).
This issue was introduced by https://github.com/graphql/graphiql/pull/1651/files where it has changed the logger to post to
stdout
for non-Error messages:This doesn't work for
stream
method asstdio
is reserved for communicating with the LSP client.@acao
The text was updated successfully, but these errors were encountered: