Upon receiving a workspace/symbol request, the server panic if Capabilities.WorkspaceSymbolClientCapabilities.symbolKind isn't set by the client during initialize

[shellwhale]

Language Server Version

v0.31.4

Terraform Version

Terraform v1.5.4 on windows_amd64, Terraform v1.5.4 on linux_amd64

Client Version

terraform-ls/internal/langserver/handlers/workspace_symbol_test.go@TestLangServer_workspace_symbol_basic

Terraform Configuration

# This the content in the workspace_symbol_test.go, but it's irrelevant to the bug.
provider "github" {}

Steps to Reproduce

1. Open terraform-ls/internal/langserver/handlers/workspace_symbol_test.go
2. Go to the TestLangServer_workspace_symbol_basic function
3. Remove the whole "symbol" property (delete lines 44 included to 55 included)
4. Run the test go test -timeout 30s -run ^TestLangServer_workspace_symbol_basic$ github.com/hashicorp/terraform-ls/internal/langserver/handlers

It is worth mentioning that the error also occurs upon receiving a textDocument/documentSymbol request. There is also a test for it in terraform-ls/internal/langserver/handlers/symbols_test.go. The error is reproducible by removing lines 44 (included) to 58 (included).

Expected Behavior

Expected some form of error handling to prevent the server crash inferred by the client and inform the user that some values should be set. Or have some default values that prevent the server crash. I'm not familiar enough with the Language Server Protocol Specification to know which one is the way to go.

Actual Behavior

The server panics with the following message to the console :

panic: runtime error: invalid memory address or nil pointer dereference

Gist

https://gist.github.com/shellwhale/a7d2f84e92838d4b1e4a01d921eebc99

Workarounds

No response

References

No response

Help Wanted

• I'm interested in contributing a fix myself

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

[radeksimko]

Hi @shellwhale
While panic isn't a desirable behaviour and is technically a violation of the LSP spec, I am curious if this in practice affects any users (i.e. is reproducible outside of a test)?

The spec says the following about the optional valueSet:

   * If this property is not present the client only supports
   * the symbol kinds from `File` to `Array` as defined in
   * the initial version of the protocol.

We'll have to find what these are and maybe encode these kinds into a slice of defaults we can then pass in cases where valueSet and/or symbolKind aren't present.

I can look into a patch but I'd still like to establish the real-world impact.

[shellwhale]

(i.e. is reproducible outside of a test)

Well, I was personally impacted! I was trying out to develop a client for the first time and came across the issue. I lost quite a bit of time wondering what could I be doing wrong.

I did not include my own client code in the issue for the sake of simplicity, I used the test so that the bug is easily reproducible.

I saw the symbolKind values are here in the spec, so in our case a default slice like you said with values 1 and 18 would be sufficient I think.

I also just noticed that they are defined here

So is that what we are looking for?

var defaultSymbols protocol.SymbolKind = []protocol.SymbolKind{protocol.File, protocol.Array}

I'd gladly make a PR for this but I'm not sure yet where we should put the default slice into.

[radeksimko]

Understood, to clarify - I wasn't implying this isn't worth fixing, I am just trying to assess the severity/impact of the bug so we can prioritise it alongside other work.

Also thank you for the report and for finding the bug.

I'd gladly make a PR for this but I'm not sure yet where we should put the default slice into.

That would be most appreciated, thanks for the offer!

... is that what we are looking for?

var defaultSymbols protocol.SymbolKind = []protocol.SymbolKind{protocol.File, protocol.Array}

Yes, pretty much. The way I would interpret the comment in the spec

symbol kinds from File to Array as defined in the initial version of the protocol

is all kinds between File and Array, i.e.

var defaultSymbols = []protocol.SymbolKind{
	protocol.File,
	protocol.Module,
	protocol.Namespace,
	protocol.Package,
	protocol.Class,
	protocol.Method,
	protocol.Property,
	protocol.Field,
	protocol.Constructor,
	protocol.Enum,
	protocol.Interface,
	protocol.Function,
	protocol.Variable,
	protocol.Constant,
	protocol.String,
	protocol.Number,
	protocol.Boolean,
	protocol.Array,
}

[jpaju]

Hi! I'm encountering this issue consistently when using the Helix editor. This makes using the LSP quite hard because it will crash on every workspace/symbol and textDocument/documentSymbol, disabling all other LSP functionality.

Here are the requests from the client that crash the LSP:

{
  "jsonrpc": "2.0",
  "method": "workspace/symbol",
  "params": { "query": "" },
  "id": 4
}

{
  "jsonrpc": "2.0",
  "method": "textDocument/documentSymbol",
  "params": {
    "textDocument": {
      "uri": "file:<filepath>.tf"
    }
  },
  "id": 1
}

[dbanck]

We just released a new version of the language server 0.32.7. This fixes the reported bug.

Thank you for the report @shellwhale

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.