Skip to content

Commit 372a7ee

Browse files
EhabYEhabY
committed
Fix subscriptions, components, and watchers not disposed on extension deactivation
1 parent 67e85e6 commit 372a7ee

File tree

6 files changed

+352
-313
lines changed

6 files changed

+352
-313
lines changed

‎src/api/coderApi.ts‎

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
type Workspace,
88
type WorkspaceAgent,
99
} from "coder/site/src/api/typesGenerated";
10-
import { type WorkspaceConfiguration } from "vscode";
10+
import * as vscode from "vscode";
1111
import { type ClientOptions } from "ws";
1212

1313
import { CertificateError } from "../error";
@@ -33,17 +33,12 @@ import { createHttpAgent } from "./utils";
3333

3434
const coderSessionTokenHeader = "Coder-Session-Token";
3535

36-
type WorkspaceConfigurationProvider = () => WorkspaceConfiguration;
37-
3836
/**
3937
* Unified API class that includes both REST API methods from the base Api class
4038
* and WebSocket methods for real-time functionality.
4139
*/
4240
export class CoderApi extends Api {
43-
private constructor(
44-
private readonly output: Logger,
45-
private readonly configProvider: WorkspaceConfigurationProvider,
46-
) {
41+
private constructor(private readonly output: Logger) {
4742
super();
4843
}
4944

@@ -55,15 +50,14 @@ export class CoderApi extends Api {
5550
baseUrl: string,
5651
token: string | undefined,
5752
output: Logger,
58-
configProvider: WorkspaceConfigurationProvider,
5953
): CoderApi {
60-
const client = new CoderApi(output, configProvider);
54+
const client = new CoderApi(output);
6155
client.setHost(baseUrl);
6256
if (token) {
6357
client.setSessionToken(token);
6458
}
6559

66-
setupInterceptors(client, baseUrl, output, configProvider);
60+
setupInterceptors(client, baseUrl, output);
6761
return client;
6862
}
6963

@@ -127,7 +121,7 @@ export class CoderApi extends Api {
127121
coderSessionTokenHeader
128122
] as string | undefined;
129123

130-
const httpAgent = createHttpAgent(this.configProvider());
124+
const httpAgent = createHttpAgent(vscode.workspace.getConfiguration());
131125
const webSocket = new OneWayWebSocket<TData>({
132126
location: baseUrl,
133127
...configs,
@@ -174,14 +168,13 @@ function setupInterceptors(
174168
client: CoderApi,
175169
baseUrl: string,
176170
output: Logger,
177-
configProvider: WorkspaceConfigurationProvider,
178171
): void {
179-
addLoggingInterceptors(client.getAxiosInstance(), output, configProvider);
172+
addLoggingInterceptors(client.getAxiosInstance(), output);
180173

181174
client.getAxiosInstance().interceptors.request.use(async (config) => {
182175
const headers = await getHeaders(
183176
baseUrl,
184-
getHeaderCommand(configProvider()),
177+
getHeaderCommand(vscode.workspace.getConfiguration()),
185178
output,
186179
);
187180
// Add headers from the header command.
@@ -192,7 +185,7 @@ function setupInterceptors(
192185
// Configure proxy and TLS.
193186
// Note that by default VS Code overrides the agent. To prevent this, set
194187
// `http.proxySupport` to `on` or `off`.
195-
const agent = createHttpAgent(configProvider());
188+
const agent = createHttpAgent(vscode.workspace.getConfiguration());
196189
config.httpsAgent = agent;
197190
config.httpAgent = agent;
198191
config.proxy = false;
@@ -209,38 +202,35 @@ function setupInterceptors(
209202
);
210203
}
211204

212-
function addLoggingInterceptors(
213-
client: AxiosInstance,
214-
logger: Logger,
215-
configProvider: WorkspaceConfigurationProvider,
216-
) {
205+
function addLoggingInterceptors(client: AxiosInstance, logger: Logger) {
217206
client.interceptors.request.use(
218207
(config) => {
219208
const configWithMeta = config as RequestConfigWithMeta;
220209
configWithMeta.metadata = createRequestMeta();
221-
logRequest(logger, configWithMeta, getLogLevel(configProvider()));
210+
logRequest(logger, configWithMeta, getLogLevel());
222211
return config;
223212
},
224213
(error: unknown) => {
225-
logError(logger, error, getLogLevel(configProvider()));
214+
logError(logger, error, getLogLevel());
226215
return Promise.reject(error);
227216
},
228217
);
229218

230219
client.interceptors.response.use(
231220
(response) => {
232-
logResponse(logger, response, getLogLevel(configProvider()));
221+
logResponse(logger, response, getLogLevel());
233222
return response;
234223
},
235224
(error: unknown) => {
236-
logError(logger, error, getLogLevel(configProvider()));
225+
logError(logger, error, getLogLevel());
237226
return Promise.reject(error);
238227
},
239228
);
240229
}
241230

242-
function getLogLevel(cfg: WorkspaceConfiguration): HttpClientLogLevel {
243-
const logLevelStr = cfg
231+
function getLogLevel(): HttpClientLogLevel {
232+
const logLevelStr = vscode.workspace
233+
.getConfiguration()
244234
.get(
245235
"coder.httpClientLogLevel",
246236
HttpClientLogLevel[HttpClientLogLevel.BASIC],

‎src/commands.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,7 @@ export class Commands {
260260
token: string,
261261
isAutologin: boolean,
262262
): Promise<{ user: User; token: string } | null> {
263-
const client = CoderApi.create(url, token, this.logger, () =>
264-
vscode.workspace.getConfiguration(),
265-
);
263+
const client = CoderApi.create(url, token, this.logger);
266264
if (!needToken(vscode.workspace.getConfiguration())) {
267265
try {
268266
const user = await client.getAuthenticatedUser();

‎src/core/container.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { SecretsManager } from "./secretsManager";
1111
* Service container for dependency injection.
1212
* Centralizes the creation and management of all core services.
1313
*/
14-
export class ServiceContainer {
14+
export class ServiceContainer implements vscode.Disposable {
1515
private readonly logger: vscode.LogOutputChannel;
1616
private readonly pathResolver: PathResolver;
1717
private readonly mementoManager: MementoManager;

‎src/extension.ts‎

Lines changed: 85 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
5454
}
5555

5656
const serviceContainer = new ServiceContainer(ctx, vscodeProposed);
57+
ctx.subscriptions.push(serviceContainer);
58+
5759
const output = serviceContainer.getLogger();
5860
const mementoManager = serviceContainer.getMementoManager();
5961
const secretsManager = serviceContainer.getSecretsManager();
@@ -69,7 +71,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
6971
url || "",
7072
await secretsManager.getSessionToken(),
7173
output,
72-
() => vscode.workspace.getConfiguration(),
7374
);
7475

7576
const myWorkspacesProvider = new WorkspaceProvider(
@@ -78,33 +79,47 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
7879
output,
7980
5,
8081
);
82+
ctx.subscriptions.push(myWorkspacesProvider);
83+
8184
const allWorkspacesProvider = new WorkspaceProvider(
8285
WorkspaceQuery.All,
8386
client,
8487
output,
8588
);
89+
ctx.subscriptions.push(allWorkspacesProvider);
8690

8791
// createTreeView, unlike registerTreeDataProvider, gives us the tree view API
8892
// (so we can see when it is visible) but otherwise they have the same effect.
8993
const myWsTree = vscode.window.createTreeView("myWorkspaces", {
9094
treeDataProvider: myWorkspacesProvider,
9195
});
96+
ctx.subscriptions.push(myWsTree);
9297
myWorkspacesProvider.setVisibility(myWsTree.visible);
93-
myWsTree.onDidChangeVisibility((event) => {
94-
myWorkspacesProvider.setVisibility(event.visible);
95-
});
98+
myWsTree.onDidChangeVisibility(
99+
(event) => {
100+
myWorkspacesProvider.setVisibility(event.visible);
101+
},
102+
undefined,
103+
ctx.subscriptions,
104+
);
96105

97106
const allWsTree = vscode.window.createTreeView("allWorkspaces", {
98107
treeDataProvider: allWorkspacesProvider,
99108
});
109+
ctx.subscriptions.push(allWsTree);
100110
allWorkspacesProvider.setVisibility(allWsTree.visible);
101-
allWsTree.onDidChangeVisibility((event) => {
102-
allWorkspacesProvider.setVisibility(event.visible);
103-
});
111+
allWsTree.onDidChangeVisibility(
112+
(event) => {
113+
allWorkspacesProvider.setVisibility(event.visible);
114+
},
115+
undefined,
116+
ctx.subscriptions,
117+
);
104118

105119
// Handle vscode:// URIs.
106-
vscode.window.registerUriHandler({
120+
const uriHandler = vscode.window.registerUriHandler({
107121
handleUri: async (uri) => {
122+
const cliManager = serviceContainer.getCliManager();
108123
const params = new URLSearchParams(uri.query);
109124
if (uri.path === "/open") {
110125
const owner = params.get("owner");
@@ -250,53 +265,79 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
250265
}
251266
},
252267
});
253-
254-
const cliManager = serviceContainer.getCliManager();
268+
ctx.subscriptions.push(uriHandler);
255269

256270
// Register globally available commands. Many of these have visibility
257271
// controlled by contexts, see `when` in the package.json.
258272
const commands = new Commands(serviceContainer, client);
259-
vscode.commands.registerCommand("coder.login", commands.login.bind(commands));
260-
vscode.commands.registerCommand(
261-
"coder.logout",
262-
commands.logout.bind(commands),
273+
ctx.subscriptions.push(
274+
vscode.commands.registerCommand(
275+
"coder.login",
276+
commands.login.bind(commands),
277+
),
263278
);
264-
vscode.commands.registerCommand("coder.open", commands.open.bind(commands));
265-
vscode.commands.registerCommand(
266-
"coder.openDevContainer",
267-
commands.openDevContainer.bind(commands),
279+
ctx.subscriptions.push(
280+
vscode.commands.registerCommand(
281+
"coder.logout",
282+
commands.logout.bind(commands),
283+
),
268284
);
269-
vscode.commands.registerCommand(
270-
"coder.openFromSidebar",
271-
commands.openFromSidebar.bind(commands),
285+
ctx.subscriptions.push(
286+
vscode.commands.registerCommand("coder.open", commands.open.bind(commands)),
272287
);
273-
vscode.commands.registerCommand(
274-
"coder.openAppStatus",
275-
commands.openAppStatus.bind(commands),
288+
ctx.subscriptions.push(
289+
vscode.commands.registerCommand(
290+
"coder.openDevContainer",
291+
commands.openDevContainer.bind(commands),
292+
),
276293
);
277-
vscode.commands.registerCommand(
278-
"coder.workspace.update",
279-
commands.updateWorkspace.bind(commands),
294+
ctx.subscriptions.push(
295+
vscode.commands.registerCommand(
296+
"coder.openFromSidebar",
297+
commands.openFromSidebar.bind(commands),
298+
),
280299
);
281-
vscode.commands.registerCommand(
282-
"coder.createWorkspace",
283-
commands.createWorkspace.bind(commands),
300+
ctx.subscriptions.push(
301+
vscode.commands.registerCommand(
302+
"coder.openAppStatus",
303+
commands.openAppStatus.bind(commands),
304+
),
284305
);
285-
vscode.commands.registerCommand(
286-
"coder.navigateToWorkspace",
287-
commands.navigateToWorkspace.bind(commands),
306+
ctx.subscriptions.push(
307+
vscode.commands.registerCommand(
308+
"coder.workspace.update",
309+
commands.updateWorkspace.bind(commands),
310+
),
288311
);
289-
vscode.commands.registerCommand(
290-
"coder.navigateToWorkspaceSettings",
291-
commands.navigateToWorkspaceSettings.bind(commands),
312+
ctx.subscriptions.push(
313+
vscode.commands.registerCommand(
314+
"coder.createWorkspace",
315+
commands.createWorkspace.bind(commands),
316+
),
292317
);
293-
vscode.commands.registerCommand("coder.refreshWorkspaces", () => {
294-
myWorkspacesProvider.fetchAndRefresh();
295-
allWorkspacesProvider.fetchAndRefresh();
296-
});
297-
vscode.commands.registerCommand(
298-
"coder.viewLogs",
299-
commands.viewLogs.bind(commands),
318+
ctx.subscriptions.push(
319+
vscode.commands.registerCommand(
320+
"coder.navigateToWorkspace",
321+
commands.navigateToWorkspace.bind(commands),
322+
),
323+
);
324+
ctx.subscriptions.push(
325+
vscode.commands.registerCommand(
326+
"coder.navigateToWorkspaceSettings",
327+
commands.navigateToWorkspaceSettings.bind(commands),
328+
),
329+
);
330+
ctx.subscriptions.push(
331+
vscode.commands.registerCommand("coder.refreshWorkspaces", () => {
332+
myWorkspacesProvider.fetchAndRefresh();
333+
allWorkspacesProvider.fetchAndRefresh();
334+
}),
335+
);
336+
ctx.subscriptions.push(
337+
vscode.commands.registerCommand(
338+
"coder.viewLogs",
339+
commands.viewLogs.bind(commands),
340+
),
300341
);
301342

302343
// Since the "onResolveRemoteAuthority:ssh-remote" activation event exists
@@ -316,6 +357,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
316357
isFirstConnect,
317358
);
318359
if (details) {
360+
ctx.subscriptions.push(details);
319361
// Authenticate the plugin client which is used in the sidebar to display
320362
// workspaces belonging to this deployment.
321363
client.setHost(details.url);

0 commit comments

Comments
 (0)