taplo icon indicating copy to clipboard operation
taplo copied to clipboard

LSP crash when typing in TOML in middle of comment: "panicked at 'called `Option::unwrap()` on a `None` value'"

Open sylann opened this issue 1 year ago • 1 comments

I've tracked down a bug that causes the LSP to crash repeatedly and eventually get blocked by VSCode.

The error

Vscode logs for "Even Better TOML LSP":

 INFO registered request handler method="textDocument/foldingRange"
 INFO registered request handler method="textDocument/documentSymbol"
 INFO registered request handler method="textDocument/formatting"
 INFO registered request handler method="textDocument/completion"
 INFO registered request handler method="textDocument/hover"
 INFO registered request handler method="textDocument/documentLink"
 INFO registered request handler method="textDocument/semanticTokens/full"
 INFO registered request handler method="textDocument/prepareRename"
 INFO registered request handler method="textDocument/rename"
 INFO registered notification handler method="initialized"
 INFO registered notification handler method="textDocument/didOpen"
 INFO registered notification handler method="textDocument/didChange"
 INFO registered notification handler method="textDocument/didSave"
 INFO registered notification handler method="textDocument/didClose"
 INFO registered notification handler method="workspace/didChangeConfiguration"
 INFO registered notification handler method="workspace/didChangeWorkspaceFolders"
 INFO registered request handler method="taplo/convertToJson"
 INFO registered request handler method="taplo/convertToToml"
 INFO registered request handler method="taplo/listSchemas"
 INFO registered request handler method="taplo/associatedSchema"
 INFO registered notification handler method="taplo/associateSchema"
 WARN no notification handler registered method=$/setTrace
 WARN no notification handler registered method=$/setTrace
panicked at 'called `Option::unwrap()` on a `None` value', /home/runner/work/taplo/taplo/crates/taplo/src/util/mod.rs:169:10

Stack:

Error
    at AA.w.wbg.__wbg_new_693216e109162396 (/Users/romain/.vscode/extensions/tamasfe.even-better-toml-0.19.2/dist/server.js:2:5792)
    at wasm://wasm/00f689f6:wasm-function[5286]:0x320b3a
    at wasm://wasm/00f689f6:wasm-function[2552]:0x2dd09e
    at wasm://wasm/00f689f6:wasm-function[4322]:0x3163e4
    at wasm://wasm/00f689f6:wasm-function[4122]:0x312ad9
    at wasm://wasm/00f689f6:wasm-function[501]:0x1c9b5c
    at wasm://wasm/00f689f6:wasm-function[61]:0x103f3
    at wasm://wasm/00f689f6:wasm-function[102]:0x9c0a7
    at wasm://wasm/00f689f6:wasm-function[3144]:0x2f9465
    at wasm://wasm/00f689f6:wasm-function[74]:0x518ee


/Users/romain/.vscode/extensions/tamasfe.even-better-toml-0.19.2/dist/server.js:40
<MINIFIED JS CONTENT WOULD BE HERE>

I've checked the source code here: https://github.com/tamasfe/taplo/blob/268c8b1de41c105324b8c667460aeb5e23b0b6da/crates/taplo/src/util/mod.rs#L162C1-L170C2

pub fn join_ranges<I: IntoIterator<Item = TextRange>>(ranges: I) -> TextRange {
    ranges
        .into_iter()
        .fold(None, |ranges, range| match ranges {
            Some(r) => Some(range.cover(r)),
            None => Some(range),
        })
        .unwrap()
}

The problem is that fold is initialized with None. So if ranges is empty, unwrap is called on None.

I initially thought returning an "empty" TextRange could do the trick, but it would still need to have a relevant start. I've searched for how the function is used, and it seems like changing the return type is not ideal either.

But I don't even know if this is the real problem. I'm not familiar with this huge code base. May be something else doesn't behave in wasm or in js?

I've also tried to look how the lsp is used, but I have no experience in this. The vscode extension stuff is a bit overwhelming too.

How to reproduce

I've managed to reproduce the crash like so: In a Cargo.toml file, find a column surrounded by spaces and start typing letters. Likely to happen when editing comments, but it does happen outside of comments too.

The details of when a crash will occur is not entirely clear. I've seen some strange inconsistencies while testing. But the most likely and most consistent way is when writing in the middle of a comment, which is the worst case. Here are some examples:

  • ✅: typing here is safe
  • ❌: typing here crashes the lsp
✅# Some✅ comment✅ ❌ ✅about✅ something✅
✅[profile✅.✅ ❌ ✅dev✅]
✅opt-level✅ ✅ = ✅ 1✅

Once the LSP has crashed, I can only use the "Developer: Reload window" command to restart.

sylann avatar Jan 01 '24 20:01 sylann

Thanks for reporting this bug!

I cannot reproduce though, so I'll assume for now that this is VSCode specific. In which case there would be 2 options:

  • VSCode sends an invalid LSP command to Taplo, resulting in a broken invariant.
  • VSCode sends a valid but unusual LSP command to Taplo, demonstrating a bug.

I don't know the code enough to know whether join_ranges should always be called with non-empty range. But given there's a try_join_ranges function below, it seems that's something that was considered. However, that function is never used, so that's also interesting.

To debug further, a log of the LSP connection would be needed (or at least the last command).

ia0 avatar Jan 02 '24 09:01 ia0