Skip to content

Commit dde77f3

Browse files
authored
Reconnect WebSockets when configuration settings change (#705)
WebSockets now automatically reconnect when proxy, TLS, or header settings change: [coder.headerCommand, coder.insecure, coder.tlsCertFile, coder.tlsKeyFile, coder.tlsCaFile, coder.tlsAltHost, http.proxy, coder.proxyBypass] Closes #691
1 parent d18dc18 commit dde77f3

File tree

7 files changed

+274
-42
lines changed

7 files changed

+274
-42
lines changed

src/api/coderApi.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
import * as vscode from "vscode";
1717
import { type ClientOptions } from "ws";
1818

19+
import { watchConfigurationChanges } from "../configWatcher";
1920
import { CertificateError } from "../error";
2021
import { getHeaderCommand, getHeaders } from "../headers";
2122
import { EventStreamLogger } from "../logging/eventStreamLogger";
@@ -51,6 +52,21 @@ import { createHttpAgent } from "./utils";
5152

5253
const coderSessionTokenHeader = "Coder-Session-Token";
5354

55+
/**
56+
* Configuration settings that affect WebSocket connections.
57+
* Changes to these settings will trigger WebSocket reconnection.
58+
*/
59+
const webSocketConfigSettings = [
60+
"coder.headerCommand",
61+
"coder.insecure",
62+
"coder.tlsCertFile",
63+
"coder.tlsKeyFile",
64+
"coder.tlsCaFile",
65+
"coder.tlsAltHost",
66+
"http.proxy",
67+
"coder.proxyBypass",
68+
] as const;
69+
5470
/**
5571
* Unified API class that includes both REST API methods from the base Api class
5672
* and WebSocket methods for real-time functionality.
@@ -59,9 +75,11 @@ export class CoderApi extends Api implements vscode.Disposable {
5975
private readonly reconnectingSockets = new Set<
6076
ReconnectingWebSocket<never>
6177
>();
78+
private readonly configWatcher: vscode.Disposable;
6279

6380
private constructor(private readonly output: Logger) {
6481
super();
82+
this.configWatcher = this.watchConfigChanges();
6583
}
6684

6785
/**
@@ -133,12 +151,34 @@ export class CoderApi extends Api implements vscode.Disposable {
133151
* This clears handlers and prevents reconnection.
134152
*/
135153
dispose(): void {
154+
this.configWatcher.dispose();
136155
for (const socket of this.reconnectingSockets) {
137156
socket.close();
138157
}
139158
this.reconnectingSockets.clear();
140159
}
141160

161+
/**
162+
* Watch for configuration changes that affect WebSocket connections.
163+
* When any watched setting changes, all active WebSockets are reconnected.
164+
*/
165+
private watchConfigChanges(): vscode.Disposable {
166+
const settings = webSocketConfigSettings.map((setting) => ({
167+
setting,
168+
getValue: () => vscode.workspace.getConfiguration().get(setting),
169+
}));
170+
return watchConfigurationChanges(settings, () => {
171+
if (this.reconnectingSockets.size > 0) {
172+
this.output.info(
173+
`Configuration changed, reconnecting ${this.reconnectingSockets.size} WebSocket(s)`,
174+
);
175+
for (const socket of this.reconnectingSockets) {
176+
socket.reconnect();
177+
}
178+
}
179+
});
180+
}
181+
142182
watchInboxNotifications = async (
143183
watchTemplates: string[],
144184
watchTargets: string[],

src/configWatcher.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { isDeepStrictEqual } from "node:util";
2+
import * as vscode from "vscode";
3+
4+
export interface WatchedSetting {
5+
setting: string;
6+
getValue: () => unknown;
7+
}
8+
9+
/**
10+
* Watch for configuration changes and invoke a callback when values change.
11+
* Only fires when actual values change, not just when settings are touched.
12+
*/
13+
export function watchConfigurationChanges(
14+
settings: WatchedSetting[],
15+
onChange: (changedSettings: string[]) => void,
16+
): vscode.Disposable {
17+
const appliedValues = new Map(settings.map((s) => [s.setting, s.getValue()]));
18+
19+
return vscode.workspace.onDidChangeConfiguration((e) => {
20+
const changedSettings: string[] = [];
21+
22+
for (const { setting, getValue } of settings) {
23+
if (!e.affectsConfiguration(setting)) {
24+
continue;
25+
}
26+
27+
const newValue = getValue();
28+
29+
if (!isDeepStrictEqual(newValue, appliedValues.get(setting))) {
30+
changedSettings.push(setting);
31+
appliedValues.set(setting, newValue);
32+
}
33+
}
34+
35+
if (changedSettings.length > 0) {
36+
onChange(changedSettings);
37+
}
38+
});
39+
}

src/remote/remote.ts

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import * as jsonc from "jsonc-parser";
88
import * as fs from "node:fs/promises";
99
import * as os from "node:os";
1010
import * as path from "node:path";
11-
import { isDeepStrictEqual } from "node:util";
1211
import * as semver from "semver";
1312
import * as vscode from "vscode";
1413

@@ -23,6 +22,7 @@ import { CoderApi } from "../api/coderApi";
2322
import { needToken } from "../api/utils";
2423
import { getGlobalFlags, getGlobalFlagsRaw, getSshFlags } from "../cliConfig";
2524
import { type Commands } from "../commands";
25+
import { watchConfigurationChanges } from "../configWatcher";
2626
import { type CliManager } from "../core/cliManager";
2727
import * as cliUtils from "../core/cliUtils";
2828
import { type ServiceContainer } from "../core/container";
@@ -854,29 +854,10 @@ export class Remote {
854854
getValue: () => unknown;
855855
}>,
856856
): vscode.Disposable {
857-
// Capture applied values at setup time
858-
const appliedValues = new Map(
859-
settings.map((s) => [s.setting, s.getValue()]),
860-
);
861-
862-
return vscode.workspace.onDidChangeConfiguration((e) => {
863-
const changedTitles: string[] = [];
864-
865-
for (const { setting, title, getValue } of settings) {
866-
if (!e.affectsConfiguration(setting)) {
867-
continue;
868-
}
869-
870-
const newValue = getValue();
857+
const titleMap = new Map(settings.map((s) => [s.setting, s.title]));
871858

872-
if (!isDeepStrictEqual(newValue, appliedValues.get(setting))) {
873-
changedTitles.push(title);
874-
}
875-
}
876-
877-
if (changedTitles.length === 0) {
878-
return;
879-
}
859+
return watchConfigurationChanges(settings, (changedSettings) => {
860+
const changedTitles = changedSettings.map((s) => titleMap.get(s)!);
880861

881862
const message =
882863
changedTitles.length === 1

test/mocks/testHelpers.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ export class MockConfigurationProvider {
2020

2121
/**
2222
* Set a configuration value that will be returned by vscode.workspace.getConfiguration().get()
23+
* Automatically fires onDidChangeConfiguration event (emulating VS Code behavior).
2324
*/
2425
set(key: string, value: unknown): void {
2526
this.config.set(key, value);
27+
this.fireConfigChangeEvent(key);
2628
}
2729

2830
/**
@@ -42,6 +44,23 @@ export class MockConfigurationProvider {
4244
this.config.clear();
4345
}
4446

47+
/**
48+
* Fire configuration change event for a specific setting.
49+
*/
50+
private fireConfigChangeEvent(setting: string): void {
51+
const fireEvent = (
52+
vscode.workspace as typeof vscode.workspace & {
53+
__fireDidChangeConfiguration: (
54+
e: vscode.ConfigurationChangeEvent,
55+
) => void;
56+
}
57+
).__fireDidChangeConfiguration;
58+
59+
fireEvent({
60+
affectsConfiguration: (section: string) => section === setting,
61+
});
62+
}
63+
4564
/**
4665
* Setup the vscode.workspace.getConfiguration mock to return our values
4766
*/
@@ -63,7 +82,7 @@ export class MockConfigurationProvider {
6382
}),
6483
inspect: vi.fn(),
6584
update: vi.fn((key: string, value: unknown) => {
66-
this.config.set(getFullKey(key), value);
85+
this.set(getFullKey(key), value);
6786
return Promise.resolve();
6887
}),
6988
};

test/mocks/vscode.runtime.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,10 @@ export const workspace = {
120120
onDidChangeWorkspaceFolders: onDidChangeWorkspaceFolders.event,
121121

122122
// test-only triggers:
123-
__fireDidChangeConfiguration: onDidChangeConfiguration.fire,
124-
__fireDidChangeWorkspaceFolders: onDidChangeWorkspaceFolders.fire,
123+
__fireDidChangeConfiguration: (e: unknown) =>
124+
onDidChangeConfiguration.fire(e),
125+
__fireDidChangeWorkspaceFolders: (e: unknown) =>
126+
onDidChangeWorkspaceFolders.fire(e),
125127
};
126128

127129
export const env = {

test/unit/api/coderApi.test.ts

Lines changed: 70 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ describe("CoderApi", () => {
8787
mockConfig.set("coder.httpClientLogLevel", "BASIC");
8888
});
8989

90+
afterEach(() => {
91+
// Dispose any api created during the test to clean up config watchers
92+
api?.dispose();
93+
});
94+
9095
describe("HTTP Interceptors", () => {
9196
it("adds custom headers and HTTP agent to requests", async () => {
9297
const mockAgent = new ProxyAgent();
@@ -429,24 +434,24 @@ describe("CoderApi", () => {
429434
});
430435
});
431436

432-
describe("Reconnection on Host/Token Changes", () => {
433-
const setupAutoOpeningWebSocket = () => {
434-
const sockets: Array<Partial<Ws>> = [];
435-
vi.mocked(Ws).mockImplementation((url: string | URL) => {
436-
const mockWs = createMockWebSocket(String(url), {
437-
on: vi.fn((event, handler) => {
438-
if (event === "open") {
439-
setImmediate(() => handler());
440-
}
441-
return mockWs as Ws;
442-
}),
443-
});
444-
sockets.push(mockWs);
445-
return mockWs as Ws;
437+
const setupAutoOpeningWebSocket = () => {
438+
const sockets: Array<Partial<Ws>> = [];
439+
vi.mocked(Ws).mockImplementation((url: string | URL) => {
440+
const mockWs = createMockWebSocket(String(url), {
441+
on: vi.fn((event, handler) => {
442+
if (event === "open") {
443+
setImmediate(() => handler());
444+
}
445+
return mockWs as Ws;
446+
}),
446447
});
447-
return sockets;
448-
};
448+
sockets.push(mockWs);
449+
return mockWs as Ws;
450+
});
451+
return sockets;
452+
};
449453

454+
describe("Reconnection on Host/Token Changes", () => {
450455
it("triggers reconnection when session token changes", async () => {
451456
const sockets = setupAutoOpeningWebSocket();
452457
api = createApi(CODER_URL, AXIOS_TOKEN);
@@ -593,6 +598,55 @@ describe("CoderApi", () => {
593598
expect(sockets[0].close).toHaveBeenCalled();
594599
});
595600
});
601+
602+
describe("Configuration Change Reconnection", () => {
603+
const tick = () => new Promise((resolve) => setImmediate(resolve));
604+
605+
it("reconnects sockets when watched config value changes", async () => {
606+
mockConfig.set("coder.insecure", false);
607+
const sockets = setupAutoOpeningWebSocket();
608+
api = createApi(CODER_URL, AXIOS_TOKEN);
609+
await api.watchAgentMetadata(AGENT_ID);
610+
611+
mockConfig.set("coder.insecure", true);
612+
await tick();
613+
614+
expect(sockets[0].close).toHaveBeenCalledWith(
615+
1000,
616+
"Replacing connection",
617+
);
618+
expect(sockets).toHaveLength(2);
619+
});
620+
621+
it.each([
622+
["unchanged value", "coder.insecure", false],
623+
["unrelated setting", "unrelated.setting", "new-value"],
624+
])("does not reconnect for %s", async (_desc, key, value) => {
625+
mockConfig.set("coder.insecure", false);
626+
const sockets = setupAutoOpeningWebSocket();
627+
api = createApi(CODER_URL, AXIOS_TOKEN);
628+
await api.watchAgentMetadata(AGENT_ID);
629+
630+
mockConfig.set(key, value);
631+
await tick();
632+
633+
expect(sockets[0].close).not.toHaveBeenCalled();
634+
expect(sockets).toHaveLength(1);
635+
});
636+
637+
it("stops watching after dispose", async () => {
638+
mockConfig.set("coder.insecure", false);
639+
const sockets = setupAutoOpeningWebSocket();
640+
api = createApi(CODER_URL, AXIOS_TOKEN);
641+
await api.watchAgentMetadata(AGENT_ID);
642+
643+
api.dispose();
644+
mockConfig.set("coder.insecure", true);
645+
await tick();
646+
647+
expect(sockets).toHaveLength(1);
648+
});
649+
});
596650
});
597651

598652
const mockAdapterImpl = vi.hoisted(() => (config: Record<string, unknown>) => {

0 commit comments

Comments
 (0)