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

Quitting Helix when ocamllsp was active causes delay and exits with an error. #5400

Closed
theteachr opened this issue Jan 4, 2023 · 5 comments · Fixed by #5471
Closed

Quitting Helix when ocamllsp was active causes delay and exits with an error. #5400

theteachr opened this issue Jan 4, 2023 · 5 comments · Fixed by #5471
Labels
A-language-server Area: Language server client C-bug Category: This is a bug

Comments

@theteachr
Copy link
Contributor

When quitting the editor from an OCaml source file with ocamllsp active, there is a noticeable delay before exiting with the following error:

Error: Timed out waiting for language servers to shutdown
@kirawi
Copy link
Member

kirawi commented Jan 4, 2023

Next time, please use the bug format to help diagnose the issue. This may be a duplicate of #2059

@theteachr
Copy link
Contributor Author

Apologies. I'll update the necessary fields in my next comments. #2059 happens on :wq and not on a plain quit with :q. Seems like that was fixed, but this happens on :q also.

Reproduction Steps

  • Open an OCaml source file
  • Make sure the LSP for OCaml is up and running for the buffer
  • Quit the editor with :q
  • Takes a few seconds and exits with an error message

helix.log

helix.log

Platform

macOS 13.1 (Ventura)

Helix Version

helix 22.12 (e2a1a975)

This could be a problem at the LSP Implementation. We can close this here if that's the case.

@the-mikedavis
Copy link
Member

This part of the log is relevant:

2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "/-----------------------------------------------------------------------\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Internal error: Uncaught exception.\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Jsonrpc: json conversion failed: invalid structured value\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Raised at Jsonrpc__Import.Json.error in file \"jsonrpc/src/import.ml\", line 31, characters 23-50\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Called from Jsonrpc__Import.Option.map in file \"jsonrpc/src/import.ml\", line 7, characters 21-26\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Called from Jsonrpc.Packet.t_of_fields in file \"jsonrpc/src/jsonrpc.ml\", line 271, characters 21-77\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Called from Lsp__Io.Make.read in file \"lsp/src/io.ml\", line 108, characters 24-57\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Called from Fiber__Scheduler.exec in file \"src/fiber/scheduler.ml\", line 73, characters 8-11\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Re-raised at Stdune__Exn.raise_with_backtrace in file \"otherlibs/stdune/exn.ml\" (inlined), line 36, characters 27-56\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Called from Stdune__Exn_with_backtrace.reraise in file \"otherlibs/stdune/exn_with_backtrace.ml\", line 18, characters 33-71\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "| Called from Fiber__Scheduler.exec in file \"src/fiber/scheduler.ml\", line 73, characters 8-11\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "\\-----------------------------------------------------------------------\n"
2023-01-05T23:13:31.638 helix_lsp::transport [ERROR] err <- "\n"

That is stderr output from the language server. It looks like it can't parse the shutdown message. It will take some investigation into the ocamllsp codebase but this looks like an upstream bug to me.

@ulugbekna
Copy link

It looks like it can't parse the shutdown message. It will take some investigation into the ocamllsp codebase but this looks like an upstream bug to me.

Hi! ocaml-lsp contributor here.

If I understand correctly, helix is violating the spec by sending params field for shutdown request as null because according to JSONRPC spec params must either be a structured value or be omitted. From the spec:

params
A Structured value that holds the parameter values to be used during the invocation of the method. This member MAY be omitted.

What do you think?

cc @rgrinberg

@the-mikedavis
Copy link
Member

Ah good catch, the jsonrpc spec is pretty unambiguous about this:

If present, parameters for the rpc call MUST be provided as a Structured value

https://www.jsonrpc.org/specification#parameter_structures

It should be a small fix to correct this on our end. I'll post a PR.

@the-mikedavis the-mikedavis added C-bug Category: This is a bug A-language-server Area: Language server client labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants