Skip to content
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

Restrict the processes that can write to standard output when using stdio transport #518

Closed
jfacorro opened this issue Mar 4, 2020 · 5 comments · Fixed by #521 or #530
Closed
Assignees
Labels
bug Something isn't working

Comments

@jfacorro
Copy link
Contributor

jfacorro commented Mar 4, 2020

Describe the bug
There are some external dependencies that are sometimes writing to stdout to report warning or errors. If the server is communicating through stdio the intervention of these dependencies corrupt the messages and some clients start failing completely.

To Reproduce
Create a module foo that has a duplicated type spec like the following:

-module(foo).
-export([baz/0]).
-type bar() :: atom().
-type bar() :: atom().

-spec baz() -> bar().
baz() -> ok.

Call foo:baz() from some other module and hover over the function call.

In VS Code this will cause the client to stop being able to process any response from the server. Opening the deveoper tools (i.e. Cmd+Shift+P -> Developer: Toogle Developer Tools) will show one or more instances of the following error: Error: Header must provide a Content-Length property..

In Emacs this isn't failing, I suspect it's because the client is more forgiving and is somehow able to recover from the cocrrupted messages.

Expected behavior
Nothing other that els_stdio should be able to write to standard output.

Actual behavior
Some clients (e.g. VS Code) stop working when something else writes to standard output.

Context

  • erlang_ls version (tag/sha): 0.1.0-49-g134838a
  • Editor used: VS Code 1.42.0
  • LSP client used: Erlang LS 0.0.9
@jfacorro jfacorro added the bug Something isn't working label Mar 4, 2020
@jfacorro
Copy link
Contributor Author

jfacorro commented Mar 4, 2020

Some useful information on group leaders.

@robertoaloi
Copy link
Member

@jfacorro Really nice finding, indeed! I had a similar issue with Elvis (see #444). This could also be related to the issue some Sublime users have experienced.

@robertoaloi
Copy link
Member

Also, out of curiosity, which dependency was the offender in this case?

@jfacorro
Copy link
Contributor Author

jfacorro commented Mar 5, 2020

edoc which is used by the hover provider, is printing warnings to standard output through the usage of the edoc_report module.

@jfacorro jfacorro changed the title Restrict the processes that can write to standard output when using that transport Restrict the processes that can write to standard output when stdio that transport Mar 5, 2020
@jfacorro jfacorro changed the title Restrict the processes that can write to standard output when stdio that transport Restrict the processes that can write to standard output when using stdio transport Mar 5, 2020
@maxno-kivra
Copy link
Contributor

I wonder if you could implement your own edoc reporter and get the warnings in a good way?

@jfacorro jfacorro self-assigned this Mar 5, 2020
jfacorro added a commit that referenced this issue Mar 6, 2020
…-access

[#518] Replace group leader when using stdio transport
jfacorro added a commit that referenced this issue Mar 6, 2020
…t_application

[#518] Avoid using application:get_application/0 which relies on the group leader
jfacorro added a commit that referenced this issue Mar 6, 2020
[#518] Fix logic of when to restrict access to stdio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants