Browser extension not working on OSS repository

[beyang]

The author of this PR reached out to me to ask if I could review this change to one of our open-source repositories: https://github.com/sourcegraph/checkup/pull/109/files. In doing the PR, I found that code intelligence from Sourcegraph doesn't work. Here's the output from the browser console:

[lguychard]

These errors are surfaced from the go language server https://sourcegraph.com/github.com/sourcegraph/go-langserver@46b1224662e1c621287c2039a8db7cd67a1d83c5/-/blob/langserver/ast.go#L40:75

The positions in hover requests are correct. Using basic code intel, code intelligence works:

cc @sourcegraph/code-intel to triage

[efritz]

@keegancsmith any possibility your recent changes caused this behavior?

[keegancsmith]

Very possibly related. We can try roll-back to the older version index.docker.io/sourcegraph/lang-go:insiders@sha256:fdc94b291825648a436c656846cf274f5ce1e52c561ce6de8a11a95984c4d74a to confirm.

[efritz]

Also, did the raw API change in a meaningful way? I've seen some discussion about it but haven't followed closely.

[efritz]

Updated lang-go with hash fdc94b291825648a436c656846cf274f5ce1e52c561ce6de8a11a95984c4d74a but it seemed to display the same behavior.

[efritz]

Behavior:

• can replicate on right-hand side of diff view
• cannot replicate on left-hand side of diff view
• cannot replicate on non-PRs

[slimsag]

https://github.com/sourcegraph/sourcegraph/issues/9372 is the only work related to the raw API I am aware of.

[felixfbecker]

I've seen these errors before, but I can't find any old issues

[felixfbecker]

I've only ever seen these in Go though, never TS, which would make me think the raw API can't be at fault since it powers both

[creachadair]

I've been fumbling around trying to reproduce this. So far, what I have observed is consonant with what @efritz has seen too, viz.: Hovers from the extension for a file on the LHS of the diff (using https://github.com/sourcegraph/checkup/pull/109/files as an example) work normally, and do not report errors. On the RHS, however, two of the files that were added in that PR (execchecker.go and execchecker_test.go) report errors back from the LSP:

• "/src/github.com/sourcegraph/checkup/execchecker.go:1:1: expected 'package', found 'EOF'"
• "invalid position: /src/github.com/sourcegraph/checkup/execchecker_test.go:32:0 (character 0 (zero-based) is beyond first line boundary)"

Interestingly these errors occur even if I do not hover over those specific files (though I can't rule out that my mouse grazed them while I was scrolling; browsers are as browsers do).

I'm chewing on the hypothesis that file fetches are not working correctly, and that we're getting back either empty files (which would explain the first error) or incomplete files (which would explain the second). We've been trawling through the various layers of VFS between the language server and the filesystem, but so far not found any smoking guns.

[creachadair]

After more digging, what we see is that fetches for the new files on the RHS of the diff are returning empty content. I think one of the layers of VFS/caching/etc. may be to blame here, but we haven't isolated it yet.

[efritz]

Breaking news

This only affects PRs with added files (something is reporting zero-length files in the new rev when they shouldn't exist in the first place).

Errors are populated from this line. Related, it looks like this line returns zero-length content readers.

We suspect that we're getting particular errors (expected 'package' but found EOF) on such PRs due to a scanner error that occurs because the set of dependencies in the cached zip archive has zero-length go files.

[efritz]

I am seeing the same bad behavior locally if I revert the langserver to sourcegraph/go-langserver@9ad62c7 and the frontend to sourcegraph/sourcegraph@55f9f21d28ca0362dd20bf6798ede024e80cfba3.

@beyang have you seen this behavior in the past or is this brand new? I've been trying to triage this for over five hours and the two leads I had aren't being fruitful (it's no change to the lang-server, and it's not related to @keegancsmith's recent change to the raw API).

[efritz]

I've seen these errors before, but I can't find any old issues

I've only ever seen these in Go though, never TS, which would make me think the raw API can't be at fault since it powers both

@felix Could you provide any more context here? I'm currently at a dead end with this.

[felixfbecker]

Unfortunately not. I only remember seeing them before, and only with Go. So likely not new, and not raw API. Maybe it's some go langserver specific caching. But I've never worked on the Go langserver, sorry :/

[keegancsmith]

The weird part is if you visit the RHS repo, these errors don't happen and code-intel works. Both viewing the added file and hovering over the existing file (which triggers the error) don't give any issues.

This makes me believe it is something different between how the extension inits/communicates with the langserver on a PR vs a repo code view. An important thing to note is the PR is from a forked repo. My educated guess is it would have something to do with that.

[efritz]

The code intel team has no immediate plans to continue triaging this issue. We have de-prioritized work on language servers in general, and we have also de-prioritized work specifically on go-langserver where this bug seems to lie. As this behavior also appears on older versions of go-langserver and the frontend, this does not seem like new behavior.

I've filed sourcegraph/go-langserver#383 to track this issue in the future if there are any new updates.

An immediate solution to your problem, @beyang, is to enable LSIF on the checkup repo.