diff --git a/src/api/coderApi.ts b/src/api/coderApi.ts index 3a36ce6070..e3d0193d1c 100644 --- a/src/api/coderApi.ts +++ b/src/api/coderApi.ts @@ -45,7 +45,7 @@ import { import { ConnectionState, ReconnectingWebSocket, - type ConnectionFailureReason, + type ConnectionProblemReason, type ReconnectingWebSocketOptions, type SocketFactory, } from "../websocket/reconnectingWebSocket"; @@ -129,8 +129,8 @@ export class CoderApi extends Api implements vscode.Disposable { private readonly telemetry: TelemetryReporter, private readonly httpRequestsTelemetry: HttpRequestsTelemetry, private readonly authConfigTracker: AuthConfigTracker, - private readonly onConnectionFailure?: ( - reason: ConnectionFailureReason, + private readonly onConnectionProblem?: ( + reason: ConnectionProblemReason, route: string, ) => void, ) { @@ -153,8 +153,8 @@ export class CoderApi extends Api implements vscode.Disposable { token: string | undefined, output: Logger, telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, - onConnectionFailure?: ( - reason: ConnectionFailureReason, + onConnectionProblem?: ( + reason: ConnectionProblemReason, route: string, ) => void, ): CoderApi { @@ -165,7 +165,7 @@ export class CoderApi extends Api implements vscode.Disposable { telemetry, httpRequestsTelemetry, authConfigTracker, - onConnectionFailure, + onConnectionProblem, ); client.getAxiosInstance().defaults.timeout = DEFAULT_REQUEST_TIMEOUT_MS; client.getAxiosInstance().defaults.headers.common[BAGGAGE_HEADER] = @@ -565,7 +565,7 @@ export class CoderApi extends Api implements vscode.Disposable { } return refreshCertificates(refreshCommand, this.output); }, - onConnectionFailure: this.onConnectionFailure, + onConnectionProblem: this.onConnectionProblem, telemetry: this.telemetry, }; diff --git a/src/extension.ts b/src/extension.ts index bb32b36c65..1c226bdb6d 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -141,7 +141,7 @@ async function doActivate( deploymentSessionAuth?.token, output, telemetryService, - serviceContainer.getConnectionLogBuffer().onConnectionFailure, + serviceContainer.getConnectionLogBuffer().flushForConnectionProblem, ); ctx.subscriptions.push(client); diff --git a/src/logging/logBuffer.ts b/src/logging/logBuffer.ts index 31e3679641..f4dd5af74d 100644 --- a/src/logging/logBuffer.ts +++ b/src/logging/logBuffer.ts @@ -29,10 +29,10 @@ const MAX_BUFFERED_CHARS = 2_000_000; /** Entries replayed per channel call, so a flush is not one RPC per entry. */ const REPLAY_CHUNK = 100; -/** Replays buffered below-level log entries on a connection failure. */ +/** Replays buffered below-level log entries on a connection problem. */ export interface ConnectionLogBuffer { flush(reason: string, options?: { readonly retain?: boolean }): void; - readonly onConnectionFailure: (reason: string, route: string) => void; + readonly flushForConnectionProblem: (reason: string, route: string) => void; } interface LogEntry { @@ -63,11 +63,11 @@ export class BufferingLogger implements Logger, ConnectionLogBuffer { public readonly error = this.wrap("error"); /** - * Flush the buffer on a terminal socket failure, keyed by the + * Flush the buffer for a socket's connection problem, keyed by the * ` ` string Support greps for. Arrow property so it can be - * passed by value as the socket's failure callback. + * passed by value as the socket's `onConnectionProblem`. */ - public readonly onConnectionFailure = ( + public readonly flushForConnectionProblem = ( reason: string, route: string, ): void => { diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 002f452d4e..695a78fde9 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -297,7 +297,8 @@ export class Remote { token, this.logger, this.serviceContainer.getTelemetryService(), - this.serviceContainer.getConnectionLogBuffer().onConnectionFailure, + this.serviceContainer.getConnectionLogBuffer() + .flushForConnectionProblem, ); disposables.push(workspaceClient); diff --git a/src/websocket/reconnectingWebSocket.ts b/src/websocket/reconnectingWebSocket.ts index 4d9b6f90e7..9308e7dfae 100644 --- a/src/websocket/reconnectingWebSocket.ts +++ b/src/websocket/reconnectingWebSocket.ts @@ -111,10 +111,10 @@ function reduceState( export type SocketFactory = () => Promise>; -export type ConnectionFailureReason = ConnectionStateReason | "unreachable"; +export type ConnectionProblemReason = ConnectionStateReason | "unreachable"; -/** Default failure callback for callers that do not observe connection failures. */ -const NOOP_CONNECTION_FAILURE = (): void => undefined; +/** Default for callers that do not observe connection problems. */ +const NOOP_CONNECTION_PROBLEM = (): void => undefined; /** * Consecutive failed reconnect attempts before the buffer is flushed once and @@ -135,8 +135,8 @@ export interface ReconnectingWebSocketOptions { * Callback invoked on a terminal failure, or once per outage when the server * stays unreachable. Retrying continues in the unreachable case. */ - onConnectionFailure?: ( - reason: ConnectionFailureReason, + onConnectionProblem?: ( + reason: ConnectionProblemReason, route: string, ) => void; } @@ -206,8 +206,8 @@ export class ReconnectingWebSocket< maxBackoffMs: options.maxBackoffMs ?? 30000, jitterFactor: options.jitterFactor ?? 0.1, onCertificateRefreshNeeded: options.onCertificateRefreshNeeded, - onConnectionFailure: - options.onConnectionFailure ?? NOOP_CONNECTION_FAILURE, + onConnectionProblem: + options.onConnectionProblem ?? NOOP_CONNECTION_PROBLEM, }; this.#lastRoute = options.route; this.#backoffMs = this.#options.initialBackoffMs; @@ -332,7 +332,7 @@ export class ReconnectingWebSocket< }); this.clearCurrentSocket(options.code, options.closeReason); if (options.failure) { - this.#options.onConnectionFailure(reason, this.#route); + this.#options.onConnectionProblem(reason, this.#route); } } @@ -477,7 +477,7 @@ export class ReconnectingWebSocket< this.#route, this.#consecutiveConnectFailures, ); - this.#options.onConnectionFailure("unreachable", this.#route); + this.#options.onConnectionProblem("unreachable", this.#route); } const jitter = this.#backoffMs * this.#options.jitterFactor * (Math.random() * 2 - 1); diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index c061370675..28831de2de 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -645,7 +645,7 @@ export function createMockServiceContainer( getLogger: () => logger, getConnectionLogBuffer: (): ConnectionLogBuffer => ({ flush: () => {}, - onConnectionFailure: () => {}, + flushForConnectionProblem: () => {}, }), getSecretsManager: () => require("secretsManager", overrides.secretsManager), diff --git a/test/unit/api/coderApi.test.ts b/test/unit/api/coderApi.test.ts index 0cf3c10226..bba11d1e9d 100644 --- a/test/unit/api/coderApi.test.ts +++ b/test/unit/api/coderApi.test.ts @@ -117,14 +117,14 @@ describe("CoderApi", () => { url = CODER_URL, token = AXIOS_TOKEN, telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, - onConnectionFailure?: (reason: string) => void, + onConnectionProblem?: (reason: string) => void, ) => { return CoderApi.create( url, token, mockLogger, telemetry, - onConnectionFailure, + onConnectionProblem, ); }; @@ -577,14 +577,14 @@ describe("CoderApi", () => { }); }); - describe("connection failure callback", () => { - it("invokes onConnectionFailure on a terminal socket failure", async () => { - const onConnectionFailure = vi.fn(); + describe("connection problem callback", () => { + it("invokes onConnectionProblem on a terminal socket failure", async () => { + const onConnectionProblem = vi.fn(); const failingApi = createApi( CODER_URL, AXIOS_TOKEN, NOOP_TELEMETRY_REPORTER, - onConnectionFailure, + onConnectionProblem, ); const mockWs = createMockWebSocket( `wss://${CODER_URL.replace("https://", "")}/api/v2/workspaceagents/${AGENT_ID}/watch-metadata-ws`, @@ -600,7 +600,7 @@ describe("CoderApi", () => { wasClean: false, }); - expect(onConnectionFailure).toHaveBeenCalledWith( + expect(onConnectionProblem).toHaveBeenCalledWith( "unrecoverable_close", `/api/v2/workspaceagents/${AGENT_ID}/watch-metadata-ws`, ); @@ -675,12 +675,12 @@ describe("CoderApi", () => { }); it("treats an HTTP failure of the SSE fallback as unrecoverable", async () => { - const onConnectionFailure = vi.fn(); + const onConnectionProblem = vi.fn(); api = createApi( CODER_URL, AXIOS_TOKEN, NOOP_TELEMETRY_REPORTER, - onConnectionFailure, + onConnectionProblem, ); setupWebSocketMock( createMockWebSocket("wss://test", { @@ -700,7 +700,7 @@ describe("CoderApi", () => { const connection = await api.watchAgentMetadata(AGENT_ID); - expect(onConnectionFailure).toHaveBeenCalledWith( + expect(onConnectionProblem).toHaveBeenCalledWith( "unrecoverable_http", `/api/v2/workspaceagents/${AGENT_ID}/watch-metadata-ws`, ); diff --git a/test/unit/commands.telemetry.test.ts b/test/unit/commands.telemetry.test.ts index 43921ee375..3b5c200f4d 100644 --- a/test/unit/commands.telemetry.test.ts +++ b/test/unit/commands.telemetry.test.ts @@ -123,7 +123,7 @@ function setup(options: SetupOptions = {}) { getNetcheckPanelFactory: () => ({}) as NetcheckPanelFactory, getConnectionLogBuffer: (): ConnectionLogBuffer => ({ flush: () => {}, - onConnectionFailure: () => {}, + flushForConnectionProblem: () => {}, }), } as ServiceContainer; diff --git a/test/unit/websocket/reconnectingWebSocket.test.ts b/test/unit/websocket/reconnectingWebSocket.test.ts index 9ad55f71e5..30c0bce87b 100644 --- a/test/unit/websocket/reconnectingWebSocket.test.ts +++ b/test/unit/websocket/reconnectingWebSocket.test.ts @@ -12,7 +12,7 @@ import { import { ConnectionState, ReconnectingWebSocket, - type ConnectionFailureReason, + type ConnectionProblemReason, type SocketFactory, } from "@/websocket/reconnectingWebSocket"; import { HandshakeError } from "@/websocket/utils"; @@ -40,7 +40,7 @@ describe("ReconnectingWebSocket", () => { describe("Reconnection Logic", () => { it("automatically reconnects on abnormal closure (1006)", async () => { - const { ws, sockets, onConnectionFailure } = + const { ws, sockets, onConnectionProblem } = await createReconnectingWebSocket(); sockets[0].fireOpen(); @@ -58,7 +58,7 @@ describe("ReconnectingWebSocket", () => { expect(ws.state).toBe(ConnectionState.CONNECTED); // A transient drop that reconnects is not a terminal failure. - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); ws.close(); }); @@ -68,7 +68,7 @@ describe("ReconnectingWebSocket", () => { ])( "reconnects on a server-initiated normal closure: $name ($code)", async ({ code }) => { - const { ws, sockets, onConnectionFailure } = + const { ws, sockets, onConnectionProblem } = await createReconnectingWebSocket(); sockets[0].fireOpen(); @@ -78,7 +78,7 @@ describe("ReconnectingWebSocket", () => { await vi.advanceTimersByTimeAsync(300); expect(sockets).toHaveLength(2); // A server-initiated close is not a terminal failure. - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); ws.close(); }, @@ -90,7 +90,7 @@ describe("ReconnectingWebSocket", () => { ])( "does not reconnect on unrecoverable WebSocket close code: %i", async (code) => { - const { ws, sockets, onConnectionFailure } = + const { ws, sockets, onConnectionProblem } = await createReconnectingWebSocket(); sockets[0].fireOpen(); @@ -101,7 +101,7 @@ describe("ReconnectingWebSocket", () => { await vi.advanceTimersByTimeAsync(10000); expect(sockets).toHaveLength(1); - expect(onConnectionFailure).toHaveBeenCalledWith( + expect(onConnectionProblem).toHaveBeenCalledWith( "unrecoverable_close", expect.any(String), ); @@ -116,16 +116,16 @@ describe("ReconnectingWebSocket", () => { const factory = vi .fn>() .mockRejectedValue(new HandshakeError(statusCode)); - const onConnectionFailure = vi.fn(); - const ws = await fromFactory(factory, { onConnectionFailure }); + const onConnectionProblem = vi.fn(); + const ws = await fromFactory(factory, { onConnectionProblem }); expect(ws.state).toBe(ConnectionState.DISCONNECTED); expect(vi.getTimerCount()).toBe(0); expect(factory).toHaveBeenCalledOnce(); if (statusCode === HttpStatusCode.UNAUTHORIZED) { - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); } else { - expect(onConnectionFailure).toHaveBeenCalledExactlyOnceWith( + expect(onConnectionProblem).toHaveBeenCalledExactlyOnceWith( "unrecoverable_http", "/api/v2/test", ); @@ -138,7 +138,7 @@ describe("ReconnectingWebSocket", () => { it.each([...UNRECOVERABLE_HTTP_CODES])( "does not reconnect on unrecoverable HTTP error via error event: %i", async (statusCode) => { - const { ws, sockets, onConnectionFailure } = + const { ws, sockets, onConnectionProblem } = await createReconnectingWebSocket(); sockets[0].fireOpen(); sockets[0].fireError(new HandshakeError(statusCode)); @@ -151,9 +151,9 @@ describe("ReconnectingWebSocket", () => { expect(vi.getTimerCount()).toBe(0); if (statusCode === HttpStatusCode.UNAUTHORIZED) { - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); } else { - expect(onConnectionFailure).toHaveBeenCalledExactlyOnceWith( + expect(onConnectionProblem).toHaveBeenCalledExactlyOnceWith( "unrecoverable_http", "/api/test", ); @@ -169,8 +169,8 @@ describe("ReconnectingWebSocket", () => { .mockRejectedValueOnce(new HandshakeError(503)) .mockResolvedValueOnce(socket) .mockImplementation(() => Promise.resolve(createMockSocket())); - const onConnectionFailure = vi.fn(); - const ws = await fromFactory(factory, { onConnectionFailure }); + const onConnectionProblem = vi.fn(); + const ws = await fromFactory(factory, { onConnectionProblem }); expect(ws.state).toBe(ConnectionState.AWAITING_RETRY); expect(vi.getTimerCount()).toBe(1); @@ -183,18 +183,18 @@ describe("ReconnectingWebSocket", () => { expect(vi.getTimerCount()).toBe(1); await vi.advanceTimersToNextTimerAsync(); expect(factory).toHaveBeenCalledTimes(3); - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); ws.close(); }); it("does not read host/port digits as a status code", async () => { - const { ws, sockets, onConnectionFailure } = + const { ws, sockets, onConnectionProblem } = await createReconnectingWebSocket(); // A port ending in 404x must not be treated as HTTP 404. sockets[0].fireError(new Error("connect ECONNREFUSED 127.0.0.1:4040")); - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); // Generic connection errors retry rather than terminate. await vi.advanceTimersByTimeAsync(1000); expect(sockets.length).toBeGreaterThan(1); @@ -755,7 +755,7 @@ describe("ReconnectingWebSocket", () => { const setupRefreshTest = async (onRefresh: () => Promise) => { const sockets: MockSocket[] = []; const refreshCallback = vi.fn().mockImplementation(onRefresh); - const onConnectionFailure = vi.fn(); + const onConnectionProblem = vi.fn(); const factory = vi.fn(() => { const socket = createMockSocket(); sockets.push(socket); @@ -763,10 +763,10 @@ describe("ReconnectingWebSocket", () => { }); const ws = await fromFactory(factory, { onCertificateRefreshNeeded: refreshCallback, - onConnectionFailure, + onConnectionProblem, }); sockets[0].fireOpen(); - return { ws, sockets, refreshCallback, onConnectionFailure }; + return { ws, sockets, refreshCallback, onConnectionProblem }; }; it("reconnects after successful refresh", async () => { @@ -784,7 +784,7 @@ describe("ReconnectingWebSocket", () => { }); it("disconnects when refresh fails", async () => { - const { ws, sockets, onConnectionFailure } = await setupRefreshTest(() => + const { ws, sockets, onConnectionProblem } = await setupRefreshTest(() => Promise.resolve(false), ); @@ -794,7 +794,7 @@ describe("ReconnectingWebSocket", () => { ); expect(sockets).toHaveLength(1); - expect(onConnectionFailure).toHaveBeenCalledWith( + expect(onConnectionProblem).toHaveBeenCalledWith( "certificate_error", expect.any(String), ); @@ -850,15 +850,15 @@ describe("ReconnectingWebSocket", () => { }); }); - describe("Connection failure callback", () => { - it("does not fire onConnectionFailure on a manual disconnect", async () => { - const { ws, sockets, onConnectionFailure } = + describe("Connection problem callback", () => { + it("does not fire onConnectionProblem on a manual disconnect", async () => { + const { ws, sockets, onConnectionProblem } = await createReconnectingWebSocket(); sockets[0].fireOpen(); ws.disconnect(); - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); ws.close(); }); }); @@ -874,12 +874,12 @@ describe("ReconnectingWebSocket", () => { async function setupUnreachable( options: { telemetry?: TelemetryReporter } = {}, ) { - const onConnectionFailure = - vi.fn<(reason: ConnectionFailureReason, route: string) => void>(); + const onConnectionProblem = + vi.fn<(reason: ConnectionProblemReason, route: string) => void>(); const { ws, sockets, setFactoryError } = await createReconnectingWebSocketWithErrorControl({ ...options, - onConnectionFailure, + onConnectionProblem, initialBackoffMs: BACKOFF_MS, maxBackoffMs: BACKOFF_MS, jitterFactor: 0, @@ -913,7 +913,7 @@ describe("ReconnectingWebSocket", () => { return { ws, sockets, - onConnectionFailure, + onConnectionProblem, startOutage, failNextAttempt, failUntilFlush, @@ -922,58 +922,58 @@ describe("ReconnectingWebSocket", () => { } it("flushes once with the unreachable reason after N failed attempts", async () => { - const { ws, onConnectionFailure, startOutage, failNextAttempt } = + const { ws, onConnectionProblem, startOutage, failNextAttempt } = await setupUnreachable(); startOutage(); // attempt 1 for (let i = 0; i < FAILURES_BEFORE_FLUSH - 2; i++) { await failNextAttempt(); // through attempt N-1 } - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); await failNextAttempt(); // attempt N - expect(onConnectionFailure).toHaveBeenCalledTimes(1); - expect(onConnectionFailure).toHaveBeenCalledWith("unreachable", ROUTE); + expect(onConnectionProblem).toHaveBeenCalledTimes(1); + expect(onConnectionProblem).toHaveBeenCalledWith("unreachable", ROUTE); ws.close(); }); it("keeps retrying through a long outage without flushing again, then recovers", async () => { - const { ws, onConnectionFailure, failUntilFlush, recover } = + const { ws, onConnectionProblem, failUntilFlush, recover } = await setupUnreachable(); await failUntilFlush(); - expect(onConnectionFailure).toHaveBeenCalledTimes(1); + expect(onConnectionProblem).toHaveBeenCalledTimes(1); // A long outage (sleep, network loss) never gives up or re-flushes. await vi.advanceTimersByTimeAsync(5 * 60_000); expect(ws.state).toBe(ConnectionState.AWAITING_RETRY); - expect(onConnectionFailure).toHaveBeenCalledTimes(1); + expect(onConnectionProblem).toHaveBeenCalledTimes(1); await recover(); expect(ws.state).toBe(ConnectionState.CONNECTED); - expect(onConnectionFailure).toHaveBeenCalledTimes(1); + expect(onConnectionProblem).toHaveBeenCalledTimes(1); ws.close(); }); it("flushes again after a successful open resets the counter", async () => { - const { ws, onConnectionFailure, failUntilFlush, recover } = + const { ws, onConnectionProblem, failUntilFlush, recover } = await setupUnreachable(); await failUntilFlush(); - expect(onConnectionFailure).toHaveBeenCalledTimes(1); + expect(onConnectionProblem).toHaveBeenCalledTimes(1); await recover(); await failUntilFlush(); - expect(onConnectionFailure).toHaveBeenCalledTimes(2); + expect(onConnectionProblem).toHaveBeenCalledTimes(2); ws.close(); }); it("does not flush a transient outage that recovers before N", async () => { - const { ws, onConnectionFailure, startOutage, failNextAttempt, recover } = + const { ws, onConnectionProblem, startOutage, failNextAttempt, recover } = await setupUnreachable(); startOutage(); @@ -981,7 +981,7 @@ describe("ReconnectingWebSocket", () => { await failNextAttempt(); await recover(); - expect(onConnectionFailure).not.toHaveBeenCalled(); + expect(onConnectionProblem).not.toHaveBeenCalled(); ws.close(); }); @@ -1069,15 +1069,15 @@ function createMockSocket(): MockSocket { }; } -type ConnectionFailureSpy = ReturnType< - typeof vi.fn<(reason: ConnectionFailureReason, route: string) => void> +type ConnectionProblemSpy = ReturnType< + typeof vi.fn<(reason: ConnectionProblemReason, route: string) => void> >; interface FactoryOptions { onDispose?: () => void; onCertificateRefreshNeeded?: () => Promise; - onConnectionFailure?: ( - reason: ConnectionFailureReason, + onConnectionProblem?: ( + reason: ConnectionProblemReason, route: string, ) => void; route?: string; @@ -1092,19 +1092,19 @@ async function createReconnectingWebSocket( ): Promise<{ ws: ReconnectingWebSocket; sockets: MockSocket[]; - onConnectionFailure: ConnectionFailureSpy; + onConnectionProblem: ConnectionProblemSpy; }> { const sockets: MockSocket[] = []; - const onConnectionFailure = - vi.fn<(reason: ConnectionFailureReason, route: string) => void>(); + const onConnectionProblem = + vi.fn<(reason: ConnectionProblemReason, route: string) => void>(); const factory = vi.fn(() => { const socket = createMockSocket(); sockets.push(socket); return Promise.resolve(socket); }); - const ws = await fromFactory(factory, { ...options, onConnectionFailure }); + const ws = await fromFactory(factory, { ...options, onConnectionProblem }); expect(sockets).toHaveLength(1); - return { ws, sockets, onConnectionFailure }; + return { ws, sockets, onConnectionProblem }; } async function createReconnectingWebSocketWithErrorControl( @@ -1150,7 +1150,7 @@ async function fromFactory( route: options.route ?? "/api/v2/test", onCertificateRefreshNeeded: options.onCertificateRefreshNeeded ?? (() => Promise.resolve(false)), - onConnectionFailure: options.onConnectionFailure ?? vi.fn(), + onConnectionProblem: options.onConnectionProblem ?? vi.fn(), initialBackoffMs: options.initialBackoffMs, maxBackoffMs: options.maxBackoffMs, jitterFactor: options.jitterFactor,