helix icon indicating copy to clipboard operation
helix copied to clipboard

Add a timeout for LSP auto-format

Open antoyo opened this issue 3 years ago • 5 comments
trafficstars

Reproduction steps

I'm not sure exactly what it takes for this to happen, but I used the write command on a Rust file and the changes were not reflected automatically on disk. It could take +10 seconds.

From the logs, it looks like it might have been waiting for the LSP to do the formatting, but failed.

Environment

  • Platform: Linux
  • Terminal emulator: Alacritty
  • Helix version: helix v0.6.0
~/.cache/helix/helix.log
2022-02-09T10:24:25.775 helix_lsp::transport [ERROR] err <- "thread 'main' panicked at 'no value set for FileSourceRootQuery(FileId(8266))', /build/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.17.0-pre.2/src/input.rs:106:32\n"
2022-02-09T10:24:25.775 helix_lsp::transport [ERROR] err <- "stack backtrace:\n"
2022-02-09T10:24:25.776 helix_lsp::transport [ERROR] err <- "note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.\n"
2022-02-09T10:24:27.575 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-02-09T10:24:27.575 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-02-09T10:25:11.703 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:25:11.968 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
[snip…]
2022-02-09T10:28:18.891 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:28:38.893 helix_view::document [WARN] LSP formatting failed: request timed out
2022-02-09T10:28:38.895 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:29:25.808 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:29:25.960 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:29:27.233 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:29:29.267 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:29:47.706 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:29:49.268 helix_view::document [WARN] LSP formatting failed: request timed out
2022-02-09T10:29:49.270 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:29:55.286 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:30:07.708 helix_view::document [WARN] LSP formatting failed: request timed out
2022-02-09T10:30:07.716 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:31:04.113 helix_lsp::transport [ERROR] err: <- IO(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })
2022-02-09T10:31:04.614 helix_term::application [ERROR] Timed out waiting for language servers to shutdown

antoyo avatar Feb 09 '22 16:02 antoyo

As a workaround, you can disable auto-format for rust in your ~/.config/helix/languages.toml as shown in the docs .

You might want to run helix with the -v flag to output to a debug file and see why it is failing to format as well.

EpocSquadron avatar Feb 09 '22 20:02 EpocSquadron

Should we also have a timeout for formatting ?

sudormrfbin avatar Feb 10 '22 16:02 sudormrfbin

Should we also have a timeout for formatting ?

Definitely. But further, should the file just be written anyway? Then try auto-format and overwrite if if was successful within the timeout period?

also see https://github.com/helix-editor/helix/issues/2059#issuecomment-1137796413

hazeycode avatar May 25 '22 20:05 hazeycode

@antoyo This definitely looks like there's an issue with your LSP. However, this does suggest that in general, it would be good to add the ability to add timeouts to the async jobs. I don't know if there are any type landmines here, but it could be simple to just add a Duration member to Job and a with_timeout method.

Should we just rename this issue to track that work? I'd be happy to pick that up after my open PR is closed, I expect it would be a quick change.

dead10ck avatar Aug 10 '22 00:08 dead10ck

Is there someone working on this issue at the moment ? If not I can try to fix it as the solution suggested by @dead10ck seems reasonable and relatively easy to implement.

ValouBambou avatar May 20 '23 14:05 ValouBambou

I think we do actually have a timeout mechanism: https://github.com/helix-editor/helix/blob/17dd102e5cccbb2a9a0f0224af63e52f3dab846b/helix-lsp/src/client.rs#L443-L448

We also write regardless of whether formatting was successful: https://github.com/helix-editor/helix/blob/17dd102e5cccbb2a9a0f0224af63e52f3dab846b/helix-term/src/commands.rs#L3098-L3136

kirawi avatar Jan 11 '24 17:01 kirawi

yeah the timeout is just pretty long by default (20s) and apply equally to all LSP operations. We could look at how VSCode handles that (default timeout, different timeouts for different operations, what is the default for those operations that have dedicated timeouts)

pascalkuthe avatar Jan 11 '24 17:01 pascalkuthe