Skip to content

Commit 4600567

Browse files
EhabYEhabY
authored
Fix subscriptions not disposed on extension deactivation (#605)
Also use `vscode.workspace.getConfiguration` instead of passing a provider
1 parent 66377d2 commit 4600567

File tree

7 files changed

+344
-329
lines changed

7 files changed

+344
-329
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: 77 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
5757
}
5858

5959
const serviceContainer = new ServiceContainer(ctx, vscodeProposed);
60+
ctx.subscriptions.push(serviceContainer);
61+
6062
const output = serviceContainer.getLogger();
6163
const mementoManager = serviceContainer.getMementoManager();
6264
const secretsManager = serviceContainer.getSecretsManager();
@@ -72,7 +74,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
7274
url || "",
7375
await secretsManager.getSessionToken(),
7476
output,
75-
() => vscode.workspace.getConfiguration(),
7677
);
7778

7879
const myWorkspacesProvider = new WorkspaceProvider(
@@ -81,33 +82,47 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
8182
output,
8283
5,
8384
);
85+
ctx.subscriptions.push(myWorkspacesProvider);
86+
8487
const allWorkspacesProvider = new WorkspaceProvider(
8588
WorkspaceQuery.All,
8689
client,
8790
output,
8891
);
92+
ctx.subscriptions.push(allWorkspacesProvider);
8993

9094
// createTreeView, unlike registerTreeDataProvider, gives us the tree view API
9195
// (so we can see when it is visible) but otherwise they have the same effect.
9296
const myWsTree = vscode.window.createTreeView(MY_WORKSPACES_TREE_ID, {
9397
treeDataProvider: myWorkspacesProvider,
9498
});
99+
ctx.subscriptions.push(myWsTree);
95100
myWorkspacesProvider.setVisibility(myWsTree.visible);
96-
myWsTree.onDidChangeVisibility((event) => {
97-
myWorkspacesProvider.setVisibility(event.visible);
98-
});
101+
myWsTree.onDidChangeVisibility(
102+
(event) => {
103+
myWorkspacesProvider.setVisibility(event.visible);
104+
},
105+
undefined,
106+
ctx.subscriptions,
107+
);
99108

100109
const allWsTree = vscode.window.createTreeView(ALL_WORKSPACES_TREE_ID, {
101110
treeDataProvider: allWorkspacesProvider,
102111
});
112+
ctx.subscriptions.push(allWsTree);
103113
allWorkspacesProvider.setVisibility(allWsTree.visible);
104-
allWsTree.onDidChangeVisibility((event) => {
105-
allWorkspacesProvider.setVisibility(event.visible);
106-
});
114+
allWsTree.onDidChangeVisibility(
115+
(event) => {
116+
allWorkspacesProvider.setVisibility(event.visible);
117+
},
118+
undefined,
119+
ctx.subscriptions,
120+
);
107121

108122
// Handle vscode:// URIs.
109-
vscode.window.registerUriHandler({
123+
const uriHandler = vscode.window.registerUriHandler({
110124
handleUri: async (uri) => {
125+
const cliManager = serviceContainer.getCliManager();
111126
const params = new URLSearchParams(uri.query);
112127
if (uri.path === "/open") {
113128
const owner = params.get("owner");
@@ -253,59 +268,63 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
253268
}
254269
},
255270
});
256-
257-
const cliManager = serviceContainer.getCliManager();
271+
ctx.subscriptions.push(uriHandler);
258272

259273
// Register globally available commands. Many of these have visibility
260274
// controlled by contexts, see `when` in the package.json.
261275
const commands = new Commands(serviceContainer, client);
262-
vscode.commands.registerCommand("coder.login", commands.login.bind(commands));
263-
vscode.commands.registerCommand(
264-
"coder.logout",
265-
commands.logout.bind(commands),
266-
);
267-
vscode.commands.registerCommand("coder.open", commands.open.bind(commands));
268-
vscode.commands.registerCommand(
269-
"coder.openDevContainer",
270-
commands.openDevContainer.bind(commands),
271-
);
272-
vscode.commands.registerCommand(
273-
"coder.openFromSidebar",
274-
commands.openFromSidebar.bind(commands),
275-
);
276-
vscode.commands.registerCommand(
277-
"coder.openAppStatus",
278-
commands.openAppStatus.bind(commands),
279-
);
280-
vscode.commands.registerCommand(
281-
"coder.workspace.update",
282-
commands.updateWorkspace.bind(commands),
283-
);
284-
vscode.commands.registerCommand(
285-
"coder.createWorkspace",
286-
commands.createWorkspace.bind(commands),
287-
);
288-
vscode.commands.registerCommand(
289-
"coder.navigateToWorkspace",
290-
commands.navigateToWorkspace.bind(commands),
291-
);
292-
vscode.commands.registerCommand(
293-
"coder.navigateToWorkspaceSettings",
294-
commands.navigateToWorkspaceSettings.bind(commands),
295-
);
296-
vscode.commands.registerCommand("coder.refreshWorkspaces", () => {
297-
myWorkspacesProvider.fetchAndRefresh();
298-
allWorkspacesProvider.fetchAndRefresh();
299-
});
300-
vscode.commands.registerCommand(
301-
"coder.viewLogs",
302-
commands.viewLogs.bind(commands),
303-
);
304-
vscode.commands.registerCommand("coder.searchMyWorkspaces", async () =>
305-
showTreeViewSearch(MY_WORKSPACES_TREE_ID),
306-
);
307-
vscode.commands.registerCommand("coder.searchAllWorkspaces", async () =>
308-
showTreeViewSearch(ALL_WORKSPACES_TREE_ID),
276+
ctx.subscriptions.push(
277+
vscode.commands.registerCommand(
278+
"coder.login",
279+
commands.login.bind(commands),
280+
),
281+
vscode.commands.registerCommand(
282+
"coder.logout",
283+
commands.logout.bind(commands),
284+
),
285+
vscode.commands.registerCommand("coder.open", commands.open.bind(commands)),
286+
vscode.commands.registerCommand(
287+
"coder.openDevContainer",
288+
commands.openDevContainer.bind(commands),
289+
),
290+
vscode.commands.registerCommand(
291+
"coder.openFromSidebar",
292+
commands.openFromSidebar.bind(commands),
293+
),
294+
vscode.commands.registerCommand(
295+
"coder.openAppStatus",
296+
commands.openAppStatus.bind(commands),
297+
),
298+
vscode.commands.registerCommand(
299+
"coder.workspace.update",
300+
commands.updateWorkspace.bind(commands),
301+
),
302+
vscode.commands.registerCommand(
303+
"coder.createWorkspace",
304+
commands.createWorkspace.bind(commands),
305+
),
306+
vscode.commands.registerCommand(
307+
"coder.navigateToWorkspace",
308+
commands.navigateToWorkspace.bind(commands),
309+
),
310+
vscode.commands.registerCommand(
311+
"coder.navigateToWorkspaceSettings",
312+
commands.navigateToWorkspaceSettings.bind(commands),
313+
),
314+
vscode.commands.registerCommand("coder.refreshWorkspaces", () => {
315+
myWorkspacesProvider.fetchAndRefresh();
316+
allWorkspacesProvider.fetchAndRefresh();
317+
}),
318+
vscode.commands.registerCommand(
319+
"coder.viewLogs",
320+
commands.viewLogs.bind(commands),
321+
),
322+
vscode.commands.registerCommand("coder.searchMyWorkspaces", async () =>
323+
showTreeViewSearch(MY_WORKSPACES_TREE_ID),
324+
),
325+
vscode.commands.registerCommand("coder.searchAllWorkspaces", async () =>
326+
showTreeViewSearch(ALL_WORKSPACES_TREE_ID),
327+
),
309328
);
310329

311330
// Since the "onResolveRemoteAuthority:ssh-remote" activation event exists
@@ -325,6 +344,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
325344
isFirstConnect,
326345
);
327346
if (details) {
347+
ctx.subscriptions.push(details);
328348
// Authenticate the plugin client which is used in the sidebar to display
329349
// workspaces belonging to this deployment.
330350
client.setHost(details.url);

0 commit comments

Comments
 (0)