Skip to content

Commit 4316a96

Browse files
hyangahgopherbot
authored andcommitted
src/language/goLanguageServer: improve suggestGoplsIssueReport
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 <[email protected]> Commit-Queue: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Suzy Mueller <[email protected]> Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
1 parent d833323 commit 4316a96

File tree

8 files changed

+117
-92
lines changed

8 files changed

+117
-92
lines changed

src/commands/startLanguageServer.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const languageServerStartMutex = new Mutex();
3030
export const startLanguageServer: CommandFactory = (ctx, goCtx) => {
3131
return async (reason: RestartReason = RestartReason.MANUAL) => {
3232
const goConfig = getGoConfig();
33-
const cfg = buildLanguageServerConfig(goConfig);
33+
const cfg = await buildLanguageServerConfig(goConfig);
3434

3535
if (typeof reason === 'string') {
3636
updateRestartHistory(goCtx, reason, cfg.enabled);
@@ -42,6 +42,7 @@ export const startLanguageServer: CommandFactory = (ctx, goCtx) => {
4242
if (reason === RestartReason.MANUAL) {
4343
await suggestGoplsIssueReport(
4444
goCtx,
45+
cfg,
4546
"Looks like you're about to manually restart the language server.",
4647
errorKind.manualRestart
4748
);

src/goCheck.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import path = require('path');
1111
import vscode = require('vscode');
1212
import { getGoplsConfig } from './config';
1313
import { goBuild } from './goBuild';
14-
import { buildLanguageServerConfig } from './language/goLanguageServer';
1514
import { goLint } from './goLint';
1615
import { isModSupported } from './goModules';
1716
import { diagnosticsStatusBarItem, outputChannel } from './goStatus';
@@ -68,8 +67,7 @@ export function check(
6867

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

7472
let testPromise: Thenable<boolean>;
7573
const testConfig: TestConfig = {

src/goStatus.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import vscode = require('vscode');
1111
import vscodeUri = require('vscode-uri');
1212
import { getGoConfig } from './config';
1313
import { formatGoVersion, GoEnvironmentOption, terminalCreationListener } from './goEnvironmentStatus';
14-
import { buildLanguageServerConfig, getLocalGoplsVersion } from './language/goLanguageServer';
1514
import { isGoFile } from './goMode';
1615
import { isModSupported, runGoEnv } from './goModules';
1716
import { allToolsInformation } from './goToolsInformation';
@@ -61,14 +60,16 @@ export const expandGoStatusBar: CommandFactory = (ctx, goCtx) => async () => {
6160
{ label: 'Choose Go Environment' }
6261
];
6362

64-
// Get the gopls configuration
63+
const cfg = goCtx.latestConfig;
64+
// Get the gopls configuration.
6565
const goConfig = getGoConfig();
66-
const cfg = buildLanguageServerConfig(goConfig);
67-
if (languageServerIsRunning && cfg.serverName === 'gopls') {
68-
const goplsVersion = await getLocalGoplsVersion(cfg);
66+
const goplsIsRunning = languageServerIsRunning && cfg && cfg.serverName === 'gopls';
67+
if (goplsIsRunning) {
68+
const goplsVersion = cfg.version;
6969
options.push({ label: `${languageServerIcon}Open 'gopls' trace`, description: `${goplsVersion?.version}` });
7070
}
71-
if (!languageServerIsRunning && !cfg.serverName && goConfig['useLanguageServer'] === true) {
71+
// In case gopls still need to be installed, cfg.serverName will be empty.
72+
if (!goplsIsRunning && goConfig.get('useLanguageServer') === true && cfg?.serverName === '') {
7273
options.push({
7374
label: 'Install Go Language Server',
7475
description: `${languageServerErrorIcon}'gopls' is required but missing`

0 commit comments

Comments
 (0)