x/tools/gopls: DidCreateFiles: panic in Files[i].URI.Clean()

[adonovan]

#!stacks
"gopanic" && "DocumentURI.Path"

Issue created by stacks.

Why is the deferred call to recoverHandlerPanic in serverDispatch not causing recovery??

This stack cX84Vw was reported by telemetry:

• crash/crash
• runtime.gopanic:+69,+0x167
• golang.org/x/tools/gopls/internal/protocol.DocumentURI.Path:+9,+0x3e
• golang.org/x/tools/gopls/internal/protocol.DocumentURI.Clean:+1,+0x17
• golang.org/x/tools/gopls/internal/cache.(*Session).SnapshotOf:+1,+0x44
• golang.org/x/tools/gopls/internal/cache.(*Session).FileOf:+1,+0x2a
• golang.org/x/tools/gopls/internal/server.(*server).DidCreateFiles:+7,+0x12a
• golang.org/x/tools/gopls/internal/protocol.serverDispatch:+650,+0x36db
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func4:+5,+0x84
• golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func5:+52,+0x908
• golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2,+0xc5
• golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3,+0x95
• runtime.goexit:+0,+0x0

golang.org/x/tools/gopls@v0.19.1 go1.24.4 windows/amd64 other,vscode (1)

[gabyhelp]

Related Issues

• x/tools/gopls: Definition: crash on [unsafe.Pointer] doc link #74434 (closed)
• x/tools/gopls: CodeActions: "end < start" panic #74394 (closed)
• x/tools/gopls: Hover: nil deref in parseDocLink #70635 (closed)
• x/tools/gopls: nil deref in DidCreateFiles on result of call to completion.NewFile #74057 (closed)
• x/tools/gopls: Implementation (by signatures): nil panic in dynamicFuncCallType #74305 (closed)
• x/tools/gopls: CodeAction: nil panic in findMatchingDiagnostics #71028 (closed)
• x/tools/gopls: panic in token.File.Name within PackageDocHTML (telemetry) #70149 (closed)
• x/tools/gopls: Definition: out-of-bounds panic #73797 (closed)
• x/tools/gopls: CodeActions: nil panic in refactorRewriteEliminateDotImport #74581
• x/tools/gopls: nil deref in x/telemetry/internal/counter.(*Counter).add #73890

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[skewb1k]

Could this be caused by this change in gopls/internal/protocol/protocol.go?

Even without that condition, recover() wouldn't work because it's placed inside a deferred function that is itself wrapped by another defer.
If I understand the intent correctly - to report the panic and then re-panic - shouldn't the function look like this?:

func recoverHandlerPanic(method string) {
	// Report panics in the handler goroutine.
	if x := recover(); x != nil {
		bug.Reportf("panic in %s request", method)
		panic(x)
	}
}

However, since this is a top-level handler, should we actually re-panic?

[gopherbot]

Change https://go.dev/cl/691315 mentions this issue: gopls/internal/protocol: remove obsolete recoverHandlerPanic

[adonovan]

That CL didn't change the behavior, but it does reveal the pre-existing behavior, which answers my question in the issue body; so thanks for that.

I expect the behavior change behind this issue is due to https://go.dev/cl/663295 or perhaps https://go.dev/cl/678416.