Skip to content

Commit 4b1b92b

Browse files
author
Kartik Raj
committed
Fix shell integration API reliability
1 parent 934f46d commit 4b1b92b

File tree

2 files changed

+34
-18
lines changed

2 files changed

+34
-18
lines changed

src/client/terminals/envCollectionActivation/service.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
121121
this.disposables,
122122
);
123123
const { shell } = this.applicationEnvironment;
124-
const isActive = this.shellIntegrationService.isWorking(shell);
124+
const isActive = await this.shellIntegrationService.isWorking(shell);
125125
const shellType = identifyShellFromShellPath(shell);
126126
if (!isActive && shellType !== TerminalShellType.commandPrompt) {
127127
traceWarn(`Shell integration is not active, environment activated maybe overriden by the shell.`);
@@ -139,7 +139,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
139139
location: ProgressLocation.Window,
140140
title: Interpreters.activatingTerminals,
141141
});
142-
await this._applyCollectionImpl(resource, shell);
142+
await this._applyCollectionImpl(resource, shell).catch((ex) => {
143+
traceError(`Failed to apply terminal env vars`, shell, ex);
144+
return Promise.reject(ex); // Ensures progress indicator does not disappear in case of errors, so we can catch issues faster.
145+
});
143146
this.progressService.hideProgress();
144147
}
145148

src/client/terminals/envCollectionActivation/shellIntegrationService.ts

+29-16
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors
77
import { TerminalShellType } from '../../common/terminal/types';
88
import { createDeferred, sleep } from '../../common/utils/async';
99
import { cache } from '../../common/utils/decorators';
10-
import { traceVerbose } from '../../logging';
10+
import { traceError, traceVerbose } from '../../logging';
1111
import { IShellIntegrationService } from '../types';
1212

1313
/**
@@ -30,8 +30,15 @@ export class ShellIntegrationService implements IShellIntegrationService {
3030
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
3131
) {}
3232

33-
@cache(-1, true)
3433
public async isWorking(shell: string): Promise<boolean> {
34+
return this._isWorking(shell).catch((ex) => {
35+
traceError(`Failed to determine if shell supports shell integration`, shell, ex);
36+
return false;
37+
});
38+
}
39+
40+
@cache(-1, true)
41+
public async _isWorking(shell: string): Promise<boolean> {
3542
const isEnabled = this.workspaceService
3643
.getConfiguration('terminal')
3744
.get<boolean>('integrated.shellIntegration.enabled')!;
@@ -50,19 +57,25 @@ export class ShellIntegrationService implements IShellIntegrationService {
5057
// Proposed API is not available, assume shell integration is working at this point.
5158
return true;
5259
}
53-
const disposable = onDidExecuteTerminalCommand((e) => {
54-
if (e.terminal.name === name) {
55-
deferred.resolve();
56-
}
57-
});
58-
const terminal = this.terminalManager.createTerminal({
59-
name,
60-
shellPath: shell,
61-
hideFromUser: true,
62-
});
63-
terminal.sendText(`echo ${shell}`);
64-
const success = await Promise.race([sleep(3000).then(() => false), deferred.promise.then(() => true)]);
65-
disposable.dispose();
66-
return success;
60+
try {
61+
const disposable = onDidExecuteTerminalCommand((e) => {
62+
if (e.terminal.name === name) {
63+
deferred.resolve();
64+
}
65+
});
66+
const terminal = this.terminalManager.createTerminal({
67+
name,
68+
shellPath: shell,
69+
hideFromUser: true,
70+
});
71+
terminal.sendText(`echo ${shell}`);
72+
const success = await Promise.race([sleep(3000).then(() => false), deferred.promise.then(() => true)]);
73+
disposable.dispose();
74+
return success;
75+
} catch (ex) {
76+
traceVerbose(`Proposed API is not available, failed to subscribe to onDidExecuteTerminalCommand`, ex);
77+
// Proposed API is not available, assume shell integration is working at this point.
78+
return true;
79+
}
6780
}
6881
}

0 commit comments

Comments
 (0)