Skip to content

Commit

Permalink
src/language/goLanguageServer: improve suggestGoplsIssueReport
Browse files Browse the repository at this point in the history
The extension runs suggestGoplsIssueReport when it observes
the language server client crashes. It prompts the user and
collects useful information from settings, configuration,
and the LSP server output channel ("gopls (server)")
where stderr and LSP log messages are logged, populates a
github issue template when the user agrees.

This change improves the log collection and sanitization.

* Incorrect version info and timestamp - previously they were
computed after the user chooses "Yes" on the prompt. Usually,
there is a delay between the problem occurs and the user
notices the popup. Often, vscode hides the prompt window!
It's possible that a new gopls or extension version was installed
and restarted in between. This CL makes the suggestGoplsIssueReport
accepts the configuration used when starting the crashed gopls
session, and computes the issue timestamp before prompting.

Moreover, we also compute the gopls version when the configuration
object is built. Previously, it was lazily evaluated to avoid
excessive file stats and `gopls version` process runs. In this
CL, we remove unnecessary buildLanguageServerConfig calls -
the latest config is cached in `goCtx.latestCfg`.

One side-effect of this change is `buildLanguageServerConfig`
is now async to run `gopls version`.

* Gopls's crash info is in `gopls (server)` output channel.
collectGoplsLog attempted to collect the data in a hacky way
by iterating all open documents and picking the first one that
looks like our log. Unfortunately, this doesn't work when there
are multiple extensions with output channels. Fix this bug
- recent versions of vscode now use file names that include the
channel name, so we can pin point the right output channel doc.

* The extension may trigger multiple gopls restarts back to back
because there are currently multiple vantage points for checking
for gopls update asynchronously. Such successive restarts may
be unclean and the lsp client lib classifies them as crashes.
The new session may be already up and running. This CL makes
suggestGoplsIssueReport check gopls versions (what's used in the
currently crashed session and what's the latest config the extension
saw) and prompt only if they are same. It would be nice if we
can centralize gopls install/upgrade decision making and reduce
the chance of successive, unnecessary gopls restarts. But that
is a major change and can be a separate project.

We also learned a couple of new crash log patterns. Integrate
the followings in the log scrubbing logic.

* log.Fatal - "filename.go:line ...."
* LSP 3.17 client library changed the initialization error log text.
That explains the increased in the number of empty reports after
we updated our dependency.

This change also embeds `gopls stats -anon` output. That may
reveal issues in the workspace setup. For a large project, gopls stats
may take a while. Limit the execution to 60sec.

While we are here, we also simplify the periodic gopls update check
(scheduleGoplsSuggestions). That will remove another
buildLanguageServerConfig call.

Fixes #984
Fixes #2690

Change-Id: Ib8aa2abbd5f0c812605ced13c9c93b8aa3bb94fd
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/539415
Auto-Submit: Hyang-Ah Hana Kim <hyangah@gmail.com>
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
  • Loading branch information
hyangah authored and gopherbot committed Nov 29, 2023
1 parent d833323 commit 4316a96
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 92 deletions.
3 changes: 2 additions & 1 deletion src/commands/startLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const languageServerStartMutex = new Mutex();
export const startLanguageServer: CommandFactory = (ctx, goCtx) => {
return async (reason: RestartReason = RestartReason.MANUAL) => {
const goConfig = getGoConfig();
const cfg = buildLanguageServerConfig(goConfig);
const cfg = await buildLanguageServerConfig(goConfig);

if (typeof reason === 'string') {
updateRestartHistory(goCtx, reason, cfg.enabled);
Expand All @@ -42,6 +42,7 @@ export const startLanguageServer: CommandFactory = (ctx, goCtx) => {
if (reason === RestartReason.MANUAL) {
await suggestGoplsIssueReport(
goCtx,
cfg,
"Looks like you're about to manually restart the language server.",
errorKind.manualRestart
);
Expand Down
4 changes: 1 addition & 3 deletions src/goCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import path = require('path');
import vscode = require('vscode');
import { getGoplsConfig } from './config';
import { goBuild } from './goBuild';
import { buildLanguageServerConfig } from './language/goLanguageServer';
import { goLint } from './goLint';
import { isModSupported } from './goModules';
import { diagnosticsStatusBarItem, outputChannel } from './goStatus';
Expand Down Expand Up @@ -68,8 +67,7 @@ export function check(

// If a user has enabled diagnostics via a language server,
// then we disable running build or vet to avoid duplicate errors and warnings.
const lspConfig = buildLanguageServerConfig(goConfig);
const disableBuildAndVet = lspConfig.enabled;
const disableBuildAndVet = goConfig.get('useLanguageServer');

let testPromise: Thenable<boolean>;
const testConfig: TestConfig = {
Expand Down
13 changes: 7 additions & 6 deletions src/goStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import vscode = require('vscode');
import vscodeUri = require('vscode-uri');
import { getGoConfig } from './config';
import { formatGoVersion, GoEnvironmentOption, terminalCreationListener } from './goEnvironmentStatus';
import { buildLanguageServerConfig, getLocalGoplsVersion } from './language/goLanguageServer';
import { isGoFile } from './goMode';
import { isModSupported, runGoEnv } from './goModules';
import { allToolsInformation } from './goToolsInformation';
Expand Down Expand Up @@ -61,14 +60,16 @@ export const expandGoStatusBar: CommandFactory = (ctx, goCtx) => async () => {
{ label: 'Choose Go Environment' }
];

// Get the gopls configuration
const cfg = goCtx.latestConfig;
// Get the gopls configuration.
const goConfig = getGoConfig();
const cfg = buildLanguageServerConfig(goConfig);
if (languageServerIsRunning && cfg.serverName === 'gopls') {
const goplsVersion = await getLocalGoplsVersion(cfg);
const goplsIsRunning = languageServerIsRunning && cfg && cfg.serverName === 'gopls';
if (goplsIsRunning) {
const goplsVersion = cfg.version;
options.push({ label: `${languageServerIcon}Open 'gopls' trace`, description: `${goplsVersion?.version}` });
}
if (!languageServerIsRunning && !cfg.serverName && goConfig['useLanguageServer'] === true) {
// In case gopls still need to be installed, cfg.serverName will be empty.
if (!goplsIsRunning && goConfig.get('useLanguageServer') === true && cfg?.serverName === '') {
options.push({
label: 'Install Go Language Server',
description: `${languageServerErrorIcon}'gopls' is required but missing`
Expand Down
Loading

0 comments on commit 4316a96

Please sign in to comment.