Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: syntax error in go.mod blocks progress end notification #50885

Closed
j-hui opened this issue Jan 28, 2022 · 11 comments
Closed

x/tools/gopls: syntax error in go.mod blocks progress end notification #50885

j-hui opened this issue Jan 28, 2022 · 11 comments
Labels
gopls/metadata gopls NeedsInvestigation Tools
Milestone

Comments

@j-hui
Copy link

@j-hui j-hui commented Jan 28, 2022

gopls version

$ gopls -v version
Build info
----------
golang.org/x/tools/gopls v0.7.5
    golang.org/x/tools/gopls@v0.7.5 h1:8Az52YwcFXTWPvrRomns1C0N+zlgTyyPKWvRazO9GG8=
    github.com/BurntSushi/toml@v0.4.1 h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw=
    github.com/google/go-cmp@v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.5.1 h1:OJxoQ/rynoF0dcCdI7cLPktw/hR2cueqYfjm43oqK38=
    golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
    golang.org/x/sys@v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=
    golang.org/x/text@v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
    golang.org/x/tools@v0.1.9-0.20220114220130-fd7798718afd h1:lTnuArxJC+n54TyvWUPyHhrnGxYvhSi13/aM2Ndr4bs=
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.2.1 h1:/EPr//+UMMXwMTkXvCCoaJDq8cpjMO80Ou+L4PDo2mY=
    mvdan.cc/gofumpt@v0.1.1 h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=
    mvdan.cc/xurls/v2@v2.3.0 h1:59Olnbt67UKpxF1EwVBopJvkSUBmgtb468E4GVWIZ1I=

go env

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/j-hui/.cache/go-build"
GOENV="/home/j-hui/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/j-hui/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/j-hui/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/j-hui/.local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/j-hui/.local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.6"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3553807707=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I'm working on a Neovim plugin that exposes the builtin LSP client's progress handler: https://github.com/j-hui/fidget.nvim. Several users have reported issues using it gopls.

In particular, there were two problems:

  1. Progress notifications seem to convey abnormally long messages.
  2. Some progress notifications don't seem to go away.

What did you expect to see?

  1. The notification message should just tell the user what it is working on. It should not communicate any diagnostic issues.
  2. Once a task is complete, the LSP server should notify completion, even if the task terminated upon encountering failure.

What did you see instead?

  1. Gopls notifies the client with extremely long diagnostic messages.
  2. Gopls never notifies the progress handler of completion.

We came across this issue when the long messages triggered an error where we ran into the max length for Lua's format specifier. We're currently fixing that by trucating the string, which stops Neovim from throwin an error, but this does not seem to be a satisfactory way of conveying the error.

Editor and settings

We can reproduce the issue with the following repo: https://github.com/j-hui/gopls-test

Clone that repo down, install nvim version >0.6.0, and then run:

nvim -u init.vim go.mod '+set ft=go'

In the bottom right corner:

/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] => ot...
                                                                                          ├ gopls

Notice that:

  1. The progress message is cut off.
  2. The spinner never goes away.

Fidget's logs can be found at:

~/.local/share/nvim/fidget.nvim.log

I am receiving the following messages:

[DEBUG Fri 28 Jan 2022 11:30:34 AM EST] ...xtern/fidget-nvim-go-mod-demo/fidget.nvim/lua/fidget.lua:217: Received progress notification: {
  info = {
    client_id = 1,
    method = "$/progress"
  },
  msg = {
    token = "5577006791947779410",
    value = {
      kind = "begin",
      message = "Loading packages...",
      title = "Setting up workspace"
    }
  }
}
[DEBUG Fri 28 Jan 2022 11:30:34 AM EST] ...xtern/fidget-nvim-go-mod-demo/fidget.nvim/lua/fidget.lua:217: Received progress notification: {
  info = {
    client_id = 1,
    method = "$/progress"
  },
  msg = {
    token = "5577006791947779410",
    value = {
      kind = "end",
      message = "Finished loading packages."
    }
  }
}
[DEBUG Fri 28 Jan 2022 11:30:34 AM EST] ...xtern/fidget-nvim-go-mod-demo/fidget.nvim/lua/fidget.lua:217: Received progress notification: {
  info = {
    client_id = 1,
    method = "$/progress"
  },
  msg = {
    token = "8674665223082153551",
    value = {
      kind = "begin",
      message = "/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] => other/module v1.4 \t or replace module/path [v1.2.3] => ../local/directory",
      title = "Error loading workspace"
    }
  }
}
[DEBUG Fri 28 Jan 2022 11:30:34 AM EST] ...xtern/fidget-nvim-go-mod-demo/fidget.nvim/lua/fidget.lua:217: Received progress notification: {
  info = {
    client_id = 1,
    method = "$/progress"
  },
  msg = {
    token = "8674665223082153551",
    value = {
      kind = "report",
      message = "/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] => other/module v1.4 \t or replace module/path [v1.2.3] => ../local/directory"
    }
  }
}

Notice that task 8674665223082153551 never completes, and reports an error message.

Logs

I'm not sure how useful this is for this particular issue:

[Trace - 11:38:53.588 AM] Sending request 'initialize - (1)'.
Params: {"processId":2675499,"clientInfo":{"name":"Neovim","version":"0.6.0"},"initializationOptions":{},"trace":"off","workspaceFolders":[{"uri":"file:\/\/\/home\/j-hui\/extern\/fidget-nvim-go-mod-demo","name":"\/home\/j-hui\/extern\/fidget-nvim-go-mod-demo"}],"capabilities":{"workspace":{"workspaceFolders":true,"applyEdit":true,"workspaceEdit":{"resourceOperations":["rename","create","delete"]},"configuration":true,"symbol":{"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]},"dynamicRegistration":false,"hierarchicalWorkspaceSymbolSupport":true}},"textDocument":{"codeAction":{"dataSupport":true,"dynamicRegistration":false,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","Empty","QuickFix","Refactor","RefactorExtract","RefactorInline","RefactorRewrite","Source","SourceOrganizeImports","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}},"resolveSupport":{"properties":["edit"]}},"definition":{"linkSupport":true},"typeDefinition":{"linkSupport":true},"signatureHelp":{"signatureInformation":{"parameterInformation":{"labelOffsetSupport":true},"documentationFormat":["markdown","plaintext"],"activeParameterSupport":true},"dynamicRegistration":false},"hover":{"contentFormat":["markdown","plaintext"],"dynamicRegistration":false},"implementation":{"linkSupport":true},"completion":{"contextSupport":false,"completionItem":{"snippetSupport":false,"commitCharactersSupport":false,"preselectSupport":false,"deprecatedSupport":false,"documentationFormat":["markdown","plaintext"]},"dynamicRegistration":false,"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]}},"rename":{"prepareSupport":true,"dynamicRegistration":false},"documentHighlight":{"dynamicRegistration":false},"documentSymbol":{"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]},"dynamicRegistration":false,"hierarchicalDocumentSymbolSupport":true},"synchronization":{"didSave":true,"dynamicRegistration":false,"willSave":false,"willSaveWaitUntil":false},"references":{"dynamicRegistration":false},"declaration":{"linkSupport":true},"publishDiagnostics":{"relatedInformation":true,"tagSupport":{"valueSet":[1,2]}}},"callHierarchy":{"dynamicRegistration":false},"window":{"workDoneProgress":true,"showMessage":{"messageActionItem":{"additionalPropertiesSupport":false}},"showDocument":{"support":false}}},"rootUri":"file:\/\/\/home\/j-hui\/extern\/fidget-nvim-go-mod-demo","rootPath":"\/home\/j-hui\/extern\/fidget-nvim-go-mod-demo"}


[Trace - 11:38:53.590 AM] Received response 'initialize - (1)' in 1ms.
Result: {"capabilities":{"textDocumentSync":{"openClose":true,"change":2,"save":{}},"completionProvider":{"triggerCharacters":["."],"completionItem":{}},"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","refactor.extract","refactor.rewrite","source.fixAll","source.organizeImports"]},"codeLensProvider":{},"documentLinkProvider":{},"workspaceSymbolProvider":true,"documentFormattingProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":""},"renameProvider":{"prepareProvider":true},"foldingRangeProvider":true,"executeCommandProvider":{"commands":["gopls.add_dependency","gopls.add_import","gopls.apply_fix","gopls.check_upgrades","gopls.gc_details","gopls.generate","gopls.generate_gopls_mod","gopls.go_get_package","gopls.list_known_packages","gopls.regenerate_cgo","gopls.remove_dependency","gopls.run_tests","gopls.start_debugging","gopls.test","gopls.tidy","gopls.toggle_gc_details","gopls.update_go_sum","gopls.upgrade_dependency","gopls.vendor","gopls.workspace_metadata"]},"callHierarchyProvider":true,"workspace":{"workspaceFolders":{"supported":true,"changeNotifications":"workspace/didChangeWorkspaceFolders"}}},"serverInfo":{"name":"gopls","version":"{\"path\":\"golang.org/x/tools/gopls\",\"version\":\"v0.7.5\",\"sum\":\"h1:8Az52YwcFXTWPvrRomns1C0N+zlgTyyPKWvRazO9GG8=\",\"deps\":[{\"path\":\"github.com/BurntSushi/toml\",\"version\":\"v0.4.1\",\"sum\":\"h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw=\"},{\"path\":\"github.com/google/go-cmp\",\"version\":\"v0.5.6\",\"sum\":\"h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=\"},{\"path\":\"github.com/sergi/go-diff\",\"version\":\"v1.1.0\",\"sum\":\"h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=\"},{\"path\":\"golang.org/x/mod\",\"version\":\"v0.5.1\",\"sum\":\"h1:OJxoQ/rynoF0dcCdI7cLPktw/hR2cueqYfjm43oqK38=\"},{\"path\":\"golang.org/x/sync\",\"version\":\"v0.0.0-20210220032951-036812b2e83c\",\"sum\":\"h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=\"},{\"path\":\"golang.org/x/sys\",\"version\":\"v0.0.0-20211019181941-9d821ace8654\",\"sum\":\"h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=\"},{\"path\":\"golang.org/x/text\",\"version\":\"v0.3.7\",\"sum\":\"h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=\"},{\"path\":\"golang.org/x/tools\",\"version\":\"v0.1.9-0.20220114220130-fd7798718afd\",\"sum\":\"h1:lTnuArxJC+n54TyvWUPyHhrnGxYvhSi13/aM2Ndr4bs=\"},{\"path\":\"golang.org/x/xerrors\",\"version\":\"v0.0.0-20200804184101-5ec99f83aff1\",\"sum\":\"h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=\"},{\"path\":\"honnef.co/go/tools\",\"version\":\"v0.2.1\",\"sum\":\"h1:/EPr//+UMMXwMTkXvCCoaJDq8cpjMO80Ou+L4PDo2mY=\"},{\"path\":\"mvdan.cc/gofumpt\",\"version\":\"v0.1.1\",\"sum\":\"h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=\"},{\"path\":\"mvdan.cc/xurls/v2\",\"version\":\"v2.3.0\",\"sum\":\"h1:59Olnbt67UKpxF1EwVBopJvkSUBmgtb468E4GVWIZ1I=\"}]}"}}


[Trace - 11:38:53.590 AM] Sending notification 'initialized'.
Params: {}


[Trace - 11:38:53.590 AM] Received request 'window/workDoneProgress/create - (1)'.
Params: {"token":"5577006791947779410"}


[Trace - 11:38:53.590 AM] Sending notification 'textDocument/didOpen'.
Params: {"textDocument":{"uri":"file:\/\/\/home\/j-hui\/extern\/fidget-nvim-go-mod-demo\/go.mod","text":"module demo\n\ngo 1.17\n\nreplace (\n\tgithub.com\/labstack\/echo\/v4 ..\/echo\n)\n\nrequire github.com\/labstack\/echo\/v4 v4.6.3\n\nrequire (\n\tgithub.com\/golang-jwt\/jwt v3.2.2+incompatible \/\/ indirect\n\tgithub.com\/labstack\/gommon v0.3.1 \/\/ indirect\n\tgithub.com\/mattn\/go-colorable v0.1.11 \/\/ indirect\n\tgithub.com\/mattn\/go-isatty v0.0.14 \/\/ indirect\n\tgithub.com\/valyala\/bytebufferpool v1.0.0 \/\/ indirect\n\tgithub.com\/valyala\/fasttemplate v1.2.1 \/\/ indirect\n\tgolang.org\/x\/crypto v0.0.0-20210817164053-32db794688a5 \/\/ indirect\n\tgolang.org\/x\/net v0.0.0-20210913180222-943fd674d43e \/\/ indirect\n\tgolang.org\/x\/sys v0.0.0-20211103235746-7861aae1554b \/\/ indirect\n\tgolang.org\/x\/text v0.3.7 \/\/ indirect\n\tgolang.org\/x\/time v0.0.0-20201208040808-7e3f01d25324 \/\/ indirect\n)\n","version":0,"languageId":"go"}}


[Trace - 11:38:53.592 AM] Sending response 'window/workDoneProgress/create - (1)' in 1ms.
Result: 


[Trace - 11:38:53.592 AM] Received notification '$/progress'.
Params: {"token":"5577006791947779410","value":{"kind":"begin","title":"Setting up workspace","message":"Loading packages..."}}


[Trace - 11:38:53.592 AM] Received request 'workspace/configuration - (2)'.
Params: {"items":[{"scopeUri":"file:///home/j-hui/extern/fidget-nvim-go-mod-demo","section":"gopls"}]}


[Trace - 11:38:53.594 AM] Sending response 'workspace/configuration - (2)' in 1ms.
Result: [null]


[Trace - 11:38:53.603 AM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"2022/01/28 11:38:53 copying workspace dir: /home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] =\u003e other/module v1.4\n\t or replace module/path [v1.2.3] =\u003e ../local/directory\n"}


[Trace - 11:38:53.603 AM] Received notification '$/progress'.
Params: {"token":"5577006791947779410","value":{"kind":"end","message":"Finished loading packages."}}


[Trace - 11:38:53.605 AM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2022/01/28 11:38:53 go env for /home/j-hui/extern/fidget-nvim-go-mod-demo\n(root /home/j-hui/extern/fidget-nvim-go-mod-demo)\n(go version go version go1.17.6 linux/amd64)\n(valid build configuration = true)\n(build flags: [])\nGOPRIVATE=\nGOPROXY=https://proxy.golang.org,direct\nGO111MODULE=\nGOPATH=/home/j-hui/go\nGOFLAGS=\nGONOSUMDB=\nGOINSECURE=\nGOSUMDB=sum.golang.org\nGOMOD=/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod\nGOMODCACHE=/home/j-hui/go/pkg/mod\nGONOPROXY=\nGOROOT=/home/j-hui/.local/go\nGOCACHE=/home/j-hui/.cache/go-build\n\n"}


[Trace - 11:38:53.606 AM] Received request 'window/workDoneProgress/create - (3)'.
Params: {"token":"8674665223082153551"}


[Trace - 11:38:53.606 AM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"2022/01/28 11:38:53 errors loading workspace: /home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] =\u003e other/module v1.4\n\t or replace module/path [v1.2.3] =\u003e ../local/directory\n\tsnapshot=0\n\tdirectory=file:///home/j-hui/extern/fidget-nvim-go-mod-demo\n"}


[Trace - 11:38:53.606 AM] Sending response 'window/workDoneProgress/create - (3)' in 0ms.
Result: 


[Trace - 11:38:53.606 AM] Received notification '$/progress'.
Params: {"token":"8674665223082153551","value":{"kind":"begin","title":"Error loading workspace","message":"/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] =\u003e other/module v1.4 \t or replace module/path [v1.2.3] =\u003e ../local/directory"}}


[Trace - 11:38:53.606 AM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod","diagnostics":[{"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":0}},"severity":1,"source":"go list","message":"/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] =\u003e other/module v1.4\n\t or replace module/path [v1.2.3] =\u003e ../local/directory"},{"range":{"start":{"line":5,"character":1},"end":{"line":5,"character":1}},"severity":1,"source":"syntax","message":"usage: replace module/path [v1.2.3] =\u003e other/module v1.4\n\t or replace module/path [v1.2.3] =\u003e ../local/directory"}]}


[Trace - 11:38:53.857 AM] Received notification '$/progress'.
Params: {"token":"8674665223082153551","value":{"kind":"report","message":"/home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] =\u003e other/module v1.4 \t or replace module/path [v1.2.3] =\u003e ../local/directory"}}


[Trace - 11:38:53.857 AM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"2022/01/28 11:38:53 errors loading workspace: /home/j-hui/extern/fidget-nvim-go-mod-demo/go.mod:6:2: usage: replace module/path [v1.2.3] =\u003e other/module v1.4\n\t or replace module/path [v1.2.3] =\u003e ../local/directory\n\tsnapshot=1\n\tdirectory=file:///home/j-hui/extern/fidget-nvim-go-mod-demo\n"}


[Trace - 11:38:59.323 AM] Sending request 'shutdown - (2)'.
Params: 


[Trace - 11:38:59.323 AM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2022/01/28 11:38:59 Shutdown session\n\tshutdown_session=1\n"}


[Trace - 11:38:59.323 AM] Received response 'shutdown - (2)' in 0ms.
Result: null


[Trace - 11:38:59.323 AM] Sending notification 'exit'.
Params: 
@gopherbot gopherbot added Tools gopls labels Jan 28, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jan 28, 2022
@hyangah
Copy link
Contributor

@hyangah hyangah commented Jan 28, 2022

Thanks for the report @j-hui

This was caused by syntax errors in go.mod.

replace (
	github.com/labstack/echo/v4 ../echo
)

This should be

replace (
	github.com/labstack/echo/v4 => ../echo
)

In VS Code, I saw gopls sending the progress end notification and the popup immediately going away as soon as I fixed the syntax error. So, wrt progress notification, I think gopls is working as expected.

However, there are still usability issues that need improvement.

Screen Shot 2022-01-28 at 12 03 39 PM

  1. The diagnostics & notification messages (usage: replace ...) are coming from the go command. The parse error message may be ok for the output of cli, but not so much in this case.

  2. The same message is used for three places. 2 diagnostics, and 1 notification. The go command reports only the line number (not column number) for this error, so the exact syntax error location isn't obvious.

  3. How to fix the issue is not obvious, especially for users who are not familiar with go.mod syntax. There was no code action, quick fix, or code lens for syntax error (not sure if that's possible). At lease, we should make the message clear to indicate "fix the broken go.mod file. then this notification will disappear".

@hyangah hyangah changed the title x/tools/gopls: Progress notification is overly verbose and never completes x/tools/gopls: syntax error in go.mod results in incomprehensible diagnostics/notification Jan 28, 2022
@hyangah hyangah removed this from the Unreleased milestone Jan 28, 2022
@hyangah hyangah added this to the gopls/on-deck milestone Jan 28, 2022
@hyangah hyangah added the NeedsInvestigation label Jan 28, 2022
@j-hui
Copy link
Author

@j-hui j-hui commented Jan 28, 2022

In VS Code, I saw gopls sending the progress end notification and the popup immediately goes away as soon as I fixed the syntax error. So, progress notification side, I think gopls is working as expected.

I realize this is a syntax error, but why is gopls reporting a diagnostic error in the progress notification? My understanding is that progress notifications are used to report progress, i.e., whether the LSP server is performing ongoing work. Reporting an error via an incomplete progress notification seems to convey the wrong idea about what the status of the language server.

If the intention is to show a popup message, shouldn't this be conveyed through something like the ShowMessage Notification endpoint?

@j-hui
Copy link
Author

@j-hui j-hui commented Jan 28, 2022

To be clear, this issue is not about how helpful the diagnostic for the syntax error is, but how the error is reported.

@hyangah
Copy link
Contributor

@hyangah hyangah commented Jan 28, 2022

Sorry if it wasn't clear. go.mod syntax error prevents successful workspace loading - it is a significant problem. The underlying go command couldn't provide any useful info with which gopls can do anything useful unless the go.mod file is fixed. There is currently no easy auto fix or fallback for gopls to do. So, it's sending a notification, begging for human action.

@j-hui
Copy link
Author

@j-hui j-hui commented Jan 28, 2022

Even if the workspace cannot load successfully, isn't the task still complete at that point? The way notifications are being sent gives the impression that the LSP server is performing ongoing work, even though it has already stopped (because it cannot recover). The LSP provides other facilities for reporting fatal errors to the user.

If gopls insists on sending an error message through the progress notification endpoint, it can also send a message through the work done progress endpoint, using the message field there.

@hyangah
Copy link
Contributor

@hyangah hyangah commented Jan 28, 2022

Gopls can't load the workspace but the loading blocks indefinitely, so the workspace load task (gopls's state initialization) isn't complete.
I will let @findleyr to think about the complexity of redesigning the architecture and the flow.

BTW, in VSCode, if the server stops without sending work done message, the client library clears it up.

@j-hui
Copy link
Author

@j-hui j-hui commented Jan 28, 2022

Thanks!

So far I haven't seen any other LSP servers behave this way, but I'm also unsure what is the "correct" behavior here---the LSP progress endpoint specification does not seem to make any mention of what to do if the server encounters an error.

I wasn't aware that VSCode's LSP client times out if it doesn't receive subsequent progress notifications. I can implement that in my plugin if that's the expected behavior; but do you know what the timeout is supposed to be?

@ttys3
Copy link

@ttys3 ttys3 commented Jan 29, 2022

@hyangah @j-hui
This syntax error ( github.com/labstack/echo/v4 ../echo) is intentional -_-

The situation is, during the middle of editing the go.mod file.

And in here, just to demonstrate the gopls issue.

@j-hui j-hui changed the title x/tools/gopls: syntax error in go.mod results in incomprehensible diagnostics/notification x/tools/gopls: syntax error in go.mod blocks progress end notification Jan 30, 2022
@hyangah
Copy link
Contributor

@hyangah hyangah commented Jan 31, 2022

@j-hui

I wasn't aware that VSCode's LSP client times out if it doesn't receive subsequent progress notifications. I can implement that in my plugin if that's the expected behavior; but do you know what the timeout is supposed to be?

Oh, I meant that VSCode cleans up pending progress notification if the LSP connection shuts down (either because the client decides to shut down the connection to restart the server, or because the server shuts down for whatever reasons). The LSP client shouldn't arbitrarily apply timeout.

@ttys3 That's actually a good point. If it was during the middle of editing the go.mod file, gopls should resolve and send the progress end notification as soon as the error in go.mod file is addressed. If it used showMessage, gopls cannot hide the prompt again when client fixed the syntax error. If gopls ended the progress notification as a result, gopls has to start the task again at some point to recover and resume its operation.

@findleyr
Copy link
Contributor

@findleyr findleyr commented May 10, 2022

This may be a dupe of #46930.

@findleyr findleyr added the gopls/metadata label May 17, 2022
@findleyr findleyr removed this from the gopls/later milestone May 18, 2022
@findleyr findleyr added this to the gopls/v0.9.0 milestone May 18, 2022
@findleyr
Copy link
Contributor

@findleyr findleyr commented May 18, 2022

Tentatively closing as a duplicate, so that we can consolidate investigation of the IWL progress notification on #46930. We can reopen if they turn out not to be dupes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/metadata gopls NeedsInvestigation Tools
Projects
None yet
Development

No branches or pull requests

5 participants