Review comments

Author: EhabY

src/api/coderApi.ts

@@ -448,7 +448,7 @@ export class CoderApi extends Api implements vscode.Disposable {
 			socketFactory,
 			this.output,
 			{
-				onCertificateExpired: async () => {
+				onCertificateRefreshNeeded: async () => {
 					const refreshCommand = getRefreshCommand();
 					if (!refreshCommand) {
 						return false;
@@ -500,35 +500,33 @@ function setupInterceptors(client: CoderApi, output: Logger): void {
 		async (err: unknown) => {
 			const refreshCommand = getRefreshCommand();
 			const certError = ClientCertificateError.fromError(err);
-			if (certError) {
-				if (certError.isRefreshable && refreshCommand) {
-					const config = (
-						err as {
-							config?: RequestConfigWithMeta & {
-								_certRetried?: boolean;
-							};
-						}
-					).config;
-
-					if (config && !config._certRetried) {
-						config._certRetried = true;
-
-						output.info(
-							`Client certificate error (alert ${certError.alertCode}), attempting refresh...`,
+			if (certError && certError.isRefreshable && refreshCommand) {
+				const config = (
+					err as {
+						config?: RequestConfigWithMeta & {
+							_certRetried?: boolean;
+						};
+					}
+				).config;
+
+				if (config && !config._certRetried) {
+					config._certRetried = true;
+
+					output.info(
+						`Client certificate error (alert ${certError.alertCode}), attempting refresh...`,
+					);
+					const success = await refreshCertificates(refreshCommand, output);
+					if (success) {
+						// Create new agent with refreshed certificates.
+						const agent = await createHttpAgent(
+							vscode.workspace.getConfiguration(),
 						);
-						const success = await refreshCertificates(refreshCommand, output);
-						if (success) {
-							// Create new agent with refreshed certificates.
-							const agent = await createHttpAgent(
-								vscode.workspace.getConfiguration(),
-							);
-							config.httpsAgent = agent;
-							config.httpAgent = agent;
-
-							// Retry the request.
-							output.info("Retrying request with refreshed certificates...");
-							return client.getAxiosInstance().request(config);
-						}
+						config.httpsAgent = agent;
+						config.httpAgent = agent;
+
+						// Retry the request.
+						output.info("Retrying request with refreshed certificates...");
+						return client.getAxiosInstance().request(config);
 					}
 				}
 

src/error/clientCertificateError.ts

@@ -36,15 +36,21 @@ export const CLIENT_CERT_MESSAGES: Record<CLIENT_CERT_ALERT, string> = {
 		"Access was denied using your TLS client certificate.",
 };
 
+/**
+ * Type guard to filter out reverse mappings from TypeScript numeric enums.
+ */
+function isNumericEnumEntry(
+	entry: [string, string | CLIENT_CERT_ALERT],
+): entry is [string, CLIENT_CERT_ALERT] {
+	return typeof entry[1] === "number";
+}
+
 /**
  * Patterns to match SSL alert types in error messages (case-insensitive).
  */
 const ALERT_PATTERNS: ReadonlyArray<[string, CLIENT_CERT_ALERT]> =
 	Object.entries(CLIENT_CERT_ALERT)
-		.filter(
-			(entry): entry is [string, CLIENT_CERT_ALERT] =>
-				typeof entry[1] === "number",
-		)
+		.filter(isNumericEnumEntry)
 		.map(([name, code]) => [name.toLowerCase(), code]);
 
 /**

src/websocket/reconnectingWebSocket.ts

@@ -24,18 +24,15 @@ export interface ReconnectingWebSocketOptions {
 	maxBackoffMs?: number;
 	jitterFactor?: number;
 	/** Callback invoked when a refreshable certificate error is detected. Returns true if refresh succeeded. */
-	onCertificateExpired?: () => Promise<boolean>;
+	onCertificateRefreshNeeded: () => Promise<boolean>;
 }
 
 export class ReconnectingWebSocket<
 	TData = unknown,
 > implements UnidirectionalStream<TData> {
 	readonly #socketFactory: SocketFactory<TData>;
 	readonly #logger: Logger;
-	readonly #options: Required<
-		Omit<ReconnectingWebSocketOptions, "onCertificateExpired">
-	> &
-		Pick<ReconnectingWebSocketOptions, "onCertificateExpired">;
+	readonly #options: Required<ReconnectingWebSocketOptions>;
 	readonly #eventHandlers: {
 		[K in WebSocketEventType]: Set<EventHandler<TData, K>>;
 	} = {
@@ -59,7 +56,7 @@ export class ReconnectingWebSocket<
 	private constructor(
 		socketFactory: SocketFactory<TData>,
 		logger: Logger,
-		options: ReconnectingWebSocketOptions = {},
+		options: ReconnectingWebSocketOptions,
 		onDispose?: () => void,
 	) {
 		this.#socketFactory = socketFactory;
@@ -68,7 +65,7 @@ export class ReconnectingWebSocket<
 			initialBackoffMs: options.initialBackoffMs ?? 250,
 			maxBackoffMs: options.maxBackoffMs ?? 30000,
 			jitterFactor: options.jitterFactor ?? 0.1,
-			onCertificateExpired: options.onCertificateExpired,
+			onCertificateRefreshNeeded: options.onCertificateRefreshNeeded,
 		};
 		this.#backoffMs = this.#options.initialBackoffMs;
 		this.#onDispose = onDispose;
@@ -77,7 +74,7 @@ export class ReconnectingWebSocket<
 	static async create<TData>(
 		socketFactory: SocketFactory<TData>,
 		logger: Logger,
-		options: ReconnectingWebSocketOptions = {},
+		options: ReconnectingWebSocketOptions,
 		onDispose?: () => void,
 	): Promise<ReconnectingWebSocket<TData>> {
 		const instance = new ReconnectingWebSocket<TData>(
@@ -296,14 +293,10 @@ export class ReconnectingWebSocket<
 
 	/**
 	 * Attempt to refresh certificates and return true if refresh succeeded.
-	 * Returns false if no callback configured or refresh failed.
 	 */
 	private async attemptCertificateRefresh(): Promise<boolean> {
-		if (!this.#options.onCertificateExpired) {
-			return false;
-		}
 		try {
-			return await this.#options.onCertificateExpired();
+			return await this.#options.onCertificateRefreshNeeded();
 		} catch (refreshError) {
 			this.#logger.error("Error during certificate refresh:", refreshError);
 			return false;

test/unit/websocket/reconnectingWebSocket.test.ts

@@ -94,6 +94,7 @@ describe("ReconnectingWebSocket", () => {
 				const ws = await ReconnectingWebSocket.create(
 					factory,
 					createMockLogger(),
+					{ onCertificateRefreshNeeded: () => Promise.resolve(false) },
 				);
 
 				// Should be disconnected after unrecoverable HTTP error
@@ -627,7 +628,7 @@ async function fromFactory<T>(
 	return await ReconnectingWebSocket.create(
 		factory,
 		createMockLogger(),
-		undefined,
+		{ onCertificateRefreshNeeded: () => Promise.resolve(false) },
 		onDispose,
 	);
 }