LSP: ERROR: Bug: Dictionary::operator[] used when there was no value for the given key, please report.

[fstxz]

Tested versions

• Not reproducible in v4.5.1.stable.official [f62fdbd]
• Reproducible in v4.6.dev.custom_build [0fdb93c]

System information

Godot v4.6.dev (0fdb93c) - Debian GNU/Linux 12 (bookworm) 12 on Wayland - X11 display driver, Multi-window, 1 monitor - OpenGL 3 (Compatibility) - AMD Radeon R7 360 Series (bonaire, LLVM 15.0.6, DRM 3.49, 6.1.0-40-amd64) - Intel(R) Core(TM) i5-3450 CPU @ 3.10GHz (4 threads) - 7.69 GiB memory

Issue description

Whenever I connect to Godot's LSP, I get these two errors in the console:

ERROR: Bug: Dictionary::operator[] used when there was no value for the given key, please report.
   at: operator[] (./core/variant/dictionary.cpp:136)
ERROR: Bug: Dictionary::operator[] used when there was no value for the given key, please report.
   at: operator[] (./core/variant/dictionary.cpp:136)

I am using Zed editor with zed-gdscript extension (v0.7.0)

Steps to reproduce

1. Open any project in Godot
2. Connect to Godot's LSP

Minimal reproduction project (MRP)

N/A

[AThousandShips]

This is probably not a bug, the error was added explicitly to handle incorrect usage, this is probably an error in the plugin rather than the engine, though it could be in the engine

Though some cases were covered in:

• LSP: Use safe methods to get data from dictionaries #111684

This might simply be some cases it did not catch

[fstxz]

I had to compile with debug symbols for a more useful info. For some reason, when running with a debugger I get way more of these errors, here are lines that trigger this error:

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Line 179 in c0c1c68

Variant root_uri_var = p_params["rootUri"];

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Line 180 in c0c1c68

Variant root_var = p_params["rootPath"];

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Line 196 in c0c1c68

String root_uri = p_params["rootUri"];

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Line 197 in c0c1c68

String root = p_params["rootPath"];

godot/modules/gdscript/language_server/godot_lsp.h

Line 689 in c0c1c68

uri = p_dict["uri"];

I also see a lot of these errors when launching the editor. They happen very early and aren't printed to the console.

[AThousandShips]

Are those called by the editor with information from a plugin? Or from a plugin? If it's from the plugin I'd say it's the responsibility of the plugin or LSP user to handle them, but should probably still be handled gracefully

[fstxz]

I am not sure I understand what you mean. rootUri and rootPath are initialization parameters that LSP client (in this case Zed) sends to Godot after connecting. Looking at the LSP spec, both of them are deprecated, so it's possible that Zed doesn't send them at all, but I am not sure.

[AThousandShips]

If they're deprecated that's a good reason to have them be optional, but otherwise the client should be responsible for providing the correct data, I'd say in this case though it should either use get to fetch it with a fallback, or have an error exiting using has

[HolonProduction]

#111684 only covered stuff that appears when using VSCode. There is probably more incorrect loading of optional stuff in the LSP code that only appears when the client uses a different set of features.

My rule of thumb for this would be: If it's optional (note the difference between nullable and optional. Nullable props must still be present as key value pairs, while optional props can be missing from the dict. This is well specified for LSP due to using typescript for the spec.) in the lsp spec we must use get. If the LSP spec requires it and the client does not send it, it's a client issue.

As per the spec:

• rootPath is optional, we should use get for it and fall back to an empty string. The code is written with the empty string fallback in mind anyway.

• rootUri is required by the spec. It can be null but must be present as key, if Zed does not send it it's a spec violation. The deprecation status is irrelevant for that, especially since we don't support the newer API which replaces it.

• uri is required for both TextDocumentItem and TextDocumentIdentifier (which AFAIK are the only types which this code is used for at the moment)

For the cases that are required by the spec, rather than silencing them, we should look into why exactly they fail in the first place. There might be some deeper issue there.

[fstxz]

So I tried printing p_params in GDScriptLanguageProtocol::initialize and this is what I get:

{ "processId": <null>, "rootUri": "file:///media/hdd/docs/projects/rust/gridee", "capabilities": { "workspace": { "applyEdit": true, "workspaceEdit": { "documentChanges": true, "resourceOperations": ["create", "rename", "delete"], "snippetEditSupport": true }, "didChangeConfiguration": { "dynamicRegistration": true }, "didChangeWatchedFiles": { "dynamicRegistration": true, "relativePatternSupport": true }, "symbol": { "dynamicRegistration": true }, "executeCommand": { "dynamicRegistration": true }, "workspaceFolders": true, "configuration": true, "codeLens": { "refreshSupport": true }, "fileOperations": { "dynamicRegistration": true, "didRename": true, "willRename": true }, "inlayHint": { "refreshSupport": true }, "diagnostics": { "refreshSupport": true } }, "textDocument": { "synchronization": { "dynamicRegistration": true, "didSave": true }, "completion": { "dynamicRegistration": true, "completionItem": { "snippetSupport": true, "documentationFormat": ["markdown", "plaintext"], "insertReplaceSupport": true, "resolveSupport": { "properties": ["additionalTextEdits", "command", "documentation"] }, "insertTextModeSupport": { "valueSet": [1.0, 2.0] }, "labelDetailsSupport": true }, "contextSupport": true, "insertTextMode": 2.0, "completionList": { "itemDefaults": ["commitCharacters", "editRange", "insertTextMode", "insertTextFormat", "data"] } }, "hover": { "dynamicRegistration": true, "contentFormat": ["markdown"] }, "signatureHelp": { "dynamicRegistration": true, "signatureInformation": { "documentationFormat": ["markdown", "plaintext"], "parameterInformation": { "labelOffsetSupport": true }, "activeParameterSupport": true } }, "documentSymbol": { "dynamicRegistration": true, "hierarchicalDocumentSymbolSupport": true }, "formatting": { "dynamicRegistration": true }, "rangeFormatting": { "dynamicRegistration": true }, "onTypeFormatting": { "dynamicRegistration": true }, "definition": { "dynamicRegistration": true, "linkSupport": true }, "codeAction": { "dynamicRegistration": true, "codeActionLiteralSupport": { "codeActionKind": { "valueSet": ["refactor", "quickfix", "source"] } }, "dataSupport": true, "resolveSupport": { "properties": ["kind", "diagnostics", "isPreferred", "disabled", "edit", "command"] } }, "codeLens": { "dynamicRegistration": true }, "colorProvider": { "dynamicRegistration": true }, "rename": { "dynamicRegistration": true, "prepareSupport": true, "prepareSupportDefaultBehavior": 1.0 }, "publishDiagnostics": { "relatedInformation": true, "tagSupport": { "valueSet": [1.0, 2.0] }, "versionSupport": true, "codeDescriptionSupport": true, "dataSupport": true }, "inlayHint": { "dynamicRegistration": true, "resolveSupport": { "properties": ["textEdits", "tooltip", "label.tooltip", "label.location", "label.command"] } }, "diagnostic": { "dynamicRegistration": true, "relatedDocumentSupport": true } }, "window": { "workDoneProgress": true, "showMessage": {  } }, "general": { "positionEncodings": ["utf-16"] }, "experimental": { "serverStatusNotification": true, "localDocs": true } }, "workspaceFolders": [{ "uri": "file:///media/hdd/docs/projects/rust/gridee", "name": "" }], "clientInfo": { "name": "Zed", "version": "0.209.6" } }

Sorry this isn't formatted properly, but as you can see, rootUri is there. And yet, it still complains that there is no value at that key.

Partial backtrace:

#0  Dictionary::operator[] (this=0x7fffffffc828, p_key=...) at ./core/variant/dictionary.cpp:136
#1  0x000055555972a90e in GDScriptLanguageProtocol::initialize (this=0x5555775d15c0, p_params=...)
    at ./modules/gdscript/language_server/gdscript_language_protocol.cpp:180

Actual line is 179, +1 for print_line.

[HolonProduction]

How are you determining that there is an error at all? You mentioned before, that it does not appear in the console.

Just in case: putting a breakpoint on ERR_FAIL_COND_V_MSG might break always (at least it does with lldb).

[fstxz]

Just in case: putting a breakpoint on ERR_FAIL_COND_V_MSG might break always (at least it does with lldb).

Oh, indeed, my bad. So, there are only two errors, both of them are from rootPath, not rootUri.

This

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Line 180 in c0c1c68

Variant root_var = p_params["rootPath"];

And this

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Line 197 in c0c1c68

String root = p_params["rootPath"];

[AThousandShips]

Which is missing from the passed argument, and optional, so it should be fetched with get gracefully like was done for other cases

I'd say there should be three approaches here for performance and clarity:

• Non-optional arguments we expect might be missing due to user error, and where the user can fix that, should use some error macro to fail with a clear warning when an argument is missing
• Non-optional arguments that we trust are present should just use the subscript and fail, it's less clear but it doesn't do unnecessary checks for something we consider fully trusted (this would generally be for cases where the engine passes it itself
• Optional arguments should use the get syntax and handle missing arguments gracefully