Add a timeout for LSP auto-format

[antoyo]

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/gh.neting.cc-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

[EpocSquadron]

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.

[sudormrfbin]

Should we also have a timeout for formatting ?

[hazeycode]

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 #2059 (comment)

[dead10ck]

@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.

[ValouBambou]

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.

[kirawi]

I think we do actually have a timeout mechanism:

helix/helix-lsp/src/client.rs

Lines 443 to 448 in 17dd102

	// TODO: delay other calls until initialize success
	timeout(Duration::from_secs(timeout_secs), rx.recv())
	.await
	.map_err(|_| Error::Timeout(id))? // return Timeout
	.ok_or(Error::StreamClosed)?
	}

We also write regardless of whether formatting was successful:

helix/helix-term/src/commands.rs

Lines 3098 to 3136 in 17dd102

	async fn make_format_callback(
	doc_id: DocumentId,
	doc_version: i32,
	view_id: ViewId,
	format: impl Future<Output = Result<Transaction, FormatterError>> + Send + 'static,
	write: Option<(Option<PathBuf>, bool)>,
	) -> anyhow::Result<job::Callback> {
	let format = format.await;
	let call: job::Callback = Callback::Editor(Box::new(move |editor| {
	if !editor.documents.contains_key(&doc_id) || !editor.tree.contains(view_id) {
	return;
	}
	let scrolloff = editor.config().scrolloff;
	let doc = doc_mut!(editor, &doc_id);
	let view = view_mut!(editor, view_id);
	if let Ok(format) = format {
	if doc.version() == doc_version {
	doc.apply(&format, view.id);
	doc.append_changes_to_history(view);
	doc.detect_indent_and_line_ending();
	view.ensure_cursor_in_view(doc, scrolloff);
	} else {
	log::info!("discarded formatting changes because the document changed");
	}
	}
	if let Some((path, force)) = write {
	let id = doc.id();
	if let Err(err) = editor.save(id, path, force) {
	editor.set_error(format!("Error saving: {}", err));
	}
	}
	}));
	Ok(call)
	}

[pascalkuthe]

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)