From 07adc3e22632e3406a939d598541113667a11d92 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Fri, 17 Oct 2025 12:14:02 +0300 Subject: [PATCH 1/3] Add SSE Connection and streamingFetchAdapter tests --- src/websocket/sseConnection.ts | 9 +- test/unit/api/streamingFetchAdapter.test.ts | 214 +++++++++++++ test/unit/websocket/sseConnection.test.ts | 318 ++++++++++++++++++++ 3 files changed, 536 insertions(+), 5 deletions(-) create mode 100644 test/unit/api/streamingFetchAdapter.test.ts create mode 100644 test/unit/websocket/sseConnection.test.ts diff --git a/src/websocket/sseConnection.ts b/src/websocket/sseConnection.ts index 834100aa..5a71d303 100644 --- a/src/websocket/sseConnection.ts +++ b/src/websocket/sseConnection.ts @@ -109,11 +109,10 @@ export class SseConnection implements UnidirectionalStream { } private createErrorEvent(event: Event | ErrorEvent): WsErrorEvent { - const errorMessage = - event instanceof ErrorEvent && event.message - ? event.message - : "SSE connection error"; - const error = event instanceof ErrorEvent ? event.error : undefined; + // Check for properties instead of instanceof to avoid browser-only ErrorEvent global + const eventWithMessage = event as { message?: string; error?: unknown }; + const errorMessage = eventWithMessage.message || "SSE connection error"; + const error = eventWithMessage.error; return { error: error, diff --git a/test/unit/api/streamingFetchAdapter.test.ts b/test/unit/api/streamingFetchAdapter.test.ts new file mode 100644 index 00000000..60ce9af4 --- /dev/null +++ b/test/unit/api/streamingFetchAdapter.test.ts @@ -0,0 +1,214 @@ +import { type AxiosInstance } from "axios"; +import { EventEmitter } from "events"; +import { type ReaderLike } from "eventsource"; +import { type IncomingMessage } from "http"; +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { createStreamingFetchAdapter } from "@/api/streamingFetchAdapter"; + +const TEST_URL = "https://example.com/api"; + +describe("createStreamingFetchAdapter", () => { + let mockAxios: AxiosInstance; + + beforeEach(() => { + vi.resetAllMocks(); + mockAxios = { + request: vi.fn(), + } as unknown as AxiosInstance; + }); + + describe("Request Handling", () => { + it("passes URL, signal, and responseType to axios", async () => { + const mockStream = createMockStream(); + setupAxiosResponse(mockAxios, 200, {}, mockStream); + + const adapter = createStreamingFetchAdapter(mockAxios); + const signal = new AbortController().signal; + + await adapter(TEST_URL, { signal }); + + expect(mockAxios.request).toHaveBeenCalledWith({ + url: TEST_URL, + signal, // correctly passes signal + headers: {}, + responseType: "stream", + validateStatus: expect.any(Function), + }); + }); + + it("applies headers in correct precedence order (config > init)", async () => { + const mockStream = createMockStream(); + setupAxiosResponse(mockAxios, 200, {}, mockStream); + + // Test 1: No config headers, only init headers + const adapter1 = createStreamingFetchAdapter(mockAxios); + await adapter1(TEST_URL, { + headers: { "X-Init": "init-value" }, + }); + + expect(mockAxios.request).toHaveBeenCalledWith( + expect.objectContaining({ + headers: { "X-Init": "init-value" }, + }), + ); + + // Test 2: Config headers merge with init headers + const adapter2 = createStreamingFetchAdapter(mockAxios, { + "X-Config": "config-value", + }); + await adapter2(TEST_URL, { + headers: { "X-Init": "init-value" }, + }); + + expect(mockAxios.request).toHaveBeenCalledWith( + expect.objectContaining({ + headers: { + "X-Init": "init-value", + "X-Config": "config-value", + }, + }), + ); + + // Test 3: Config headers override init headers + const adapter3 = createStreamingFetchAdapter(mockAxios, { + "X-Header": "config-value", + }); + await adapter3(TEST_URL, { + headers: { "X-Header": "init-value" }, + }); + + expect(mockAxios.request).toHaveBeenCalledWith( + expect.objectContaining({ + headers: { "X-Header": "config-value" }, + }), + ); + }); + }); + + describe("Response Properties", () => { + it("returns response with correct properties", async () => { + const mockStream = createMockStream(); + setupAxiosResponse( + mockAxios, + 200, + { "content-type": "text/event-stream" }, + mockStream, + ); + + const adapter = createStreamingFetchAdapter(mockAxios); + const response = await adapter(TEST_URL); + + expect(response.url).toBe(TEST_URL); + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe("text/event-stream"); + expect(response.headers.get("CoNtEnT-TyPe")).toBe("text/event-stream"); + expect(response.body?.getReader).toBeDefined(); + }); + + it("detects redirected requests", async () => { + const mockStream = createMockStream(); + const mockResponse = { + status: 200, + headers: {}, + data: mockStream, + request: { + res: { + responseUrl: "https://redirect.com/api", + }, + }, + }; + vi.mocked(mockAxios.request).mockResolvedValue(mockResponse); + + const adapter = createStreamingFetchAdapter(mockAxios); + const response = await adapter(TEST_URL); + + expect(response.redirected).toBe(true); + }); + }); + + describe("Stream Handling", () => { + it("enqueues data chunks from stream", async () => { + const { mockStream, reader } = await setupReaderTest(); + + const chunk1 = Buffer.from("data1"); + const chunk2 = Buffer.from("data2"); + mockStream.emit("data", chunk1); + mockStream.emit("data", chunk2); + mockStream.emit("end"); + + const result1 = await reader.read(); + expect(result1.value).toEqual(chunk1); + expect(result1.done).toBe(false); + + const result2 = await reader.read(); + expect(result2.value).toEqual(chunk2); + expect(result2.done).toBe(false); + + const result3 = await reader.read(); + // Closed after end + expect(result3.done).toBe(true); + }); + + it("propagates stream errors", async () => { + const { mockStream, reader } = await setupReaderTest(); + + const error = new Error("Stream error"); + mockStream.emit("error", error); + + await expect(reader.read()).rejects.toThrow("Stream error"); + }); + + it("handles errors after stream is closed", async () => { + const { mockStream, reader } = await setupReaderTest(); + + mockStream.emit("end"); + await reader.read(); + + // Emit events after stream is closed - should not throw + expect(() => mockStream.emit("data", Buffer.from("late"))).not.toThrow(); + expect(() => mockStream.emit("end")).not.toThrow(); + }); + + it("destroys stream on cancel", async () => { + const { mockStream, reader } = await setupReaderTest(); + + await reader.cancel(); + + expect(mockStream.destroy).toHaveBeenCalled(); + }); + }); + + async function setupReaderTest(): Promise<{ + mockStream: IncomingMessage; + reader: ReaderLike | ReadableStreamDefaultReader>; + }> { + const mockStream = createMockStream(); + setupAxiosResponse(mockAxios, 200, {}, mockStream); + + const adapter = createStreamingFetchAdapter(mockAxios); + const response = await adapter(TEST_URL); + const reader = response.body!.getReader(); + + return { mockStream, reader }; + } +}); + +function createMockStream(): IncomingMessage { + const stream = new EventEmitter() as IncomingMessage; + stream.destroy = vi.fn(); + return stream; +} + +function setupAxiosResponse( + axios: AxiosInstance, + status: number, + headers: Record, + stream: IncomingMessage, +): void { + vi.mocked(axios.request).mockResolvedValue({ + status, + headers, + data: stream, + }); +} diff --git a/test/unit/websocket/sseConnection.test.ts b/test/unit/websocket/sseConnection.test.ts new file mode 100644 index 00000000..9b2237bc --- /dev/null +++ b/test/unit/websocket/sseConnection.test.ts @@ -0,0 +1,318 @@ +import axios, { type AxiosInstance } from "axios"; +import { type ServerSentEvent } from "coder/site/src/api/typesGenerated"; +import { EventSource } from "eventsource"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { type CloseEvent, type ErrorEvent } from "ws"; + +import { type ParsedMessageEvent } from "@/websocket/eventStreamConnection"; +import { SseConnection } from "@/websocket/sseConnection"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +const TEST_URL = "https://coder.example.com"; +const API_ROUTE = "/api/v2/workspaces/123/watch"; + +vi.mock("eventsource"); +vi.mock("axios"); + +vi.mock("@/api/streamingFetchAdapter", () => ({ + createStreamingFetchAdapter: vi.fn(() => fetch), +})); + +describe("SseConnection", () => { + let mockAxios: AxiosInstance; + let mockLogger: ReturnType; + + beforeEach(() => { + vi.resetAllMocks(); + mockAxios = axios.create(); + mockLogger = createMockLogger(); + }); + + describe("URL Building", () => { + it.each([ + [undefined, `${TEST_URL}${API_ROUTE}`], + [ + { follow: "true", after: "123" }, + `${TEST_URL}${API_ROUTE}?follow=true&after=123`, + ], + [new URLSearchParams({ foo: "bar" }), `${TEST_URL}${API_ROUTE}?foo=bar`], + ])("constructs URL with %s search params", (searchParams, expectedUrl) => { + const mockES = createMockEventSource(); + setupEventSourceMock(mockES); + + const connection = new SseConnection({ + location: { protocol: "https:", host: "coder.example.com" }, + apiRoute: API_ROUTE, + searchParams, + axiosInstance: mockAxios, + logger: mockLogger, + }); + expect(connection.url).toBe(expectedUrl); + }); + }); + + describe("Event Handling", () => { + it("fires open event and supports multiple listeners", async () => { + const mockES = createMockEventSource({ + addEventListener: vi.fn((event, handler) => { + if (event === "open") { + setImmediate(() => handler(new Event("open"))); + } + }), + }); + setupEventSourceMock(mockES); + + const connection = createConnection(); + const events1: unknown[] = []; + const events2: unknown[] = []; + connection.addEventListener("open", (event) => events1.push(event)); + connection.addEventListener("open", (event) => events2.push(event)); + + await waitForNextTick(); + expect(events1).toEqual([{}]); + expect(events2).toEqual([{}]); + }); + + it("fires message event with parsed JSON and handles parse errors", async () => { + const testData = { type: "data", workspace: { status: "running" } }; + const mockES = createMockEventSource({ + addEventListener: vi.fn((event, handler) => { + if (event === "data") { + setImmediate(() => { + // Send valid JSON + handler( + new MessageEvent("data", { data: JSON.stringify(testData) }), + ); + // Send invalid JSON + handler(new MessageEvent("data", { data: "not-valid-json" })); + }); + } + }), + }); + setupEventSourceMock(mockES); + + const connection = createConnection(); + const events: ParsedMessageEvent[] = []; + connection.addEventListener("message", (event) => events.push(event)); + + await waitForNextTick(); + expect(events).toEqual([ + { + sourceEvent: { data: JSON.stringify(testData) }, + parsedMessage: { type: "data", data: testData }, + parseError: undefined, + }, + { + sourceEvent: { data: "not-valid-json" }, + parsedMessage: undefined, + parseError: expect.any(Error), + }, + ]); + }); + + it("fires error event when connection fails", async () => { + const mockES = createMockEventSource({ + addEventListener: vi.fn((event, handler) => { + if (event === "error") { + const error = { + message: "Connection failed", + error: new Error("Network error"), + }; + setImmediate(() => handler(error)); + } + }), + }); + setupEventSourceMock(mockES); + + const connection = createConnection(); + const events: ErrorEvent[] = []; + connection.addEventListener("error", (event) => events.push(event)); + + await waitForNextTick(); + expect(events).toEqual([ + { + error: expect.any(Error), + message: "Connection failed", + }, + ]); + }); + + it("fires close event when connection closes on error", async () => { + const mockES = createMockEventSource({ + addEventListener: vi.fn((event, handler) => { + if (event === "error") { + setImmediate(() => { + // Simulate EventSource behavior: state transitions to CLOSED when error occurs + (mockES as { readyState: number }).readyState = + EventSource.CLOSED; + handler(new Event("error")); + }); + } + }), + }); + setupEventSourceMock(mockES); + + const connection = createConnection(); + const events: CloseEvent[] = []; + connection.addEventListener("close", (event) => events.push(event)); + + await waitForNextTick(); + expect(events).toEqual([ + { + code: 1006, + reason: "Connection lost", + wasClean: false, + }, + ]); + }); + }); + + describe("Event Listener Management", () => { + it("removes event listener without affecting others", async () => { + const mockES = createMockEventSource({ + addEventListener: vi.fn((event, handler) => { + if (event === "data") { + setImmediate(() => + handler(new MessageEvent("data", { data: '{"test": true}' })), + ); + } + }), + }); + setupEventSourceMock(mockES); + + const connection = createConnection(); + const events: ParsedMessageEvent[] = []; + + const removedHandler = () => { + throw new Error("Removed handler should not have been called!"); + }; + const keptHandler = (event: ParsedMessageEvent) => + events.push(event); + + connection.addEventListener("message", removedHandler); + connection.addEventListener("message", keptHandler); + connection.removeEventListener("message", removedHandler); + + await waitForNextTick(); + expect(events).toHaveLength(1); + expect(mockLogger.error).not.toHaveBeenCalled(); + }); + }); + + describe("Close Handling", () => { + it.each([ + [ + undefined, + undefined, + { code: 1000, reason: "Normal closure", wasClean: true }, + ], + [ + 4000, + "Custom close", + { code: 4000, reason: "Custom close", wasClean: true }, + ], + ])( + "closes EventSource with code '%s' and reason '%s'", + (code, reason, closeEvent) => { + const mockES = createMockEventSource(); + setupEventSourceMock(mockES); + + const connection = createConnection(); + const events: CloseEvent[] = []; + connection.addEventListener("close", (event) => events.push(event)); + connection.addEventListener("open", () => {}); + + connection.close(code, reason); + expect(mockES.close).toHaveBeenCalled(); + expect(events).toEqual([closeEvent]); + }, + ); + }); + + describe("Callback Error Handling", () => { + it.each([ + ["open", new Event("open")], + ["message", new MessageEvent("data", { data: '{"test": true}' })], + ["error", new Event("error")], + ] as const)( + "logs error and continues when %s callback throws", + async (sseEvent, eventData) => { + const mockES = createMockEventSource({ + addEventListener: vi.fn((event, handler) => { + const esEvent = sseEvent === "message" ? "data" : sseEvent; + if (event === esEvent) { + setImmediate(() => handler(eventData)); + } + }), + }); + setupEventSourceMock(mockES); + + const connection = createConnection(); + const events: unknown[] = []; + + connection.addEventListener(sseEvent, () => { + throw new Error("Handler error"); + }); + connection.addEventListener(sseEvent, (event: unknown) => + events.push(event), + ); + + await waitForNextTick(); + expect(events).toHaveLength(1); + expect(mockLogger.error).toHaveBeenCalledWith( + `Error in SSE ${sseEvent} callback:`, + expect.any(Error), + ); + }, + ); + + it("completes cleanup when close callback throws", () => { + const mockES = createMockEventSource(); + setupEventSourceMock(mockES); + + const connection = createConnection(); + connection.addEventListener("close", () => { + throw new Error("Handler error"); + }); + + connection.close(); + + expect(mockES.close).toHaveBeenCalled(); + expect(mockLogger.error).toHaveBeenCalledWith( + "Error in SSE close callback:", + expect.any(Error), + ); + }); + }); + + function createConnection(): SseConnection { + return new SseConnection({ + location: { protocol: "https:", host: "coder.example.com" }, + apiRoute: API_ROUTE, + axiosInstance: mockAxios, + logger: mockLogger, + }); + } +}); + +function createMockEventSource( + overrides?: Partial, +): Partial { + return { + url: `${TEST_URL}${API_ROUTE}`, + readyState: EventSource.CONNECTING, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + close: vi.fn(), + ...overrides, + }; +} + +function setupEventSourceMock(es: Partial): void { + vi.mocked(EventSource).mockImplementation(() => es as EventSource); +} + +function waitForNextTick(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} From 0b4e1a46732e9c46e8f84ae92831467463fc6003 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 20 Oct 2025 13:36:46 +0300 Subject: [PATCH 2/3] Review comments --- test/unit/api/coderApi.test.ts | 4 +- test/unit/api/streamingFetchAdapter.test.ts | 64 ++++++----- test/unit/core/cliManager.test.ts | 5 +- test/unit/logging/utils.test.ts | 3 +- test/unit/websocket/sseConnection.test.ts | 116 +++++++++++++------- 5 files changed, 119 insertions(+), 73 deletions(-) diff --git a/test/unit/api/coderApi.test.ts b/test/unit/api/coderApi.test.ts index 0336d564..f133a72d 100644 --- a/test/unit/api/coderApi.test.ts +++ b/test/unit/api/coderApi.test.ts @@ -125,7 +125,7 @@ describe("CoderApi", () => { expect(thrownError.x509Err).toBeDefined(); }); - it("applies headers in correct precedence order (command > config > axios default)", async () => { + it("applies headers in correct precedence order (command overrides config overrides axios default)", async () => { const api = createApi(CODER_URL, AXIOS_TOKEN); // Test 1: Headers from config, default token from API creation @@ -225,7 +225,7 @@ describe("CoderApi", () => { }); }); - it("applies headers in correct precedence order (command > config > axios default)", async () => { + it("applies headers in correct precedence order (command overrides config overrides axios default)", async () => { // Test 1: Default token from API creation await api.watchBuildLogsByBuildId(BUILD_ID, []); diff --git a/test/unit/api/streamingFetchAdapter.test.ts b/test/unit/api/streamingFetchAdapter.test.ts index 60ce9af4..0ba8437b 100644 --- a/test/unit/api/streamingFetchAdapter.test.ts +++ b/test/unit/api/streamingFetchAdapter.test.ts @@ -1,25 +1,17 @@ -import { type AxiosInstance } from "axios"; -import { EventEmitter } from "events"; +import { type AxiosInstance, type AxiosResponse } from "axios"; import { type ReaderLike } from "eventsource"; -import { type IncomingMessage } from "http"; -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { EventEmitter } from "node:events"; +import { type IncomingMessage } from "node:http"; +import { describe, it, expect, vi } from "vitest"; import { createStreamingFetchAdapter } from "@/api/streamingFetchAdapter"; const TEST_URL = "https://example.com/api"; describe("createStreamingFetchAdapter", () => { - let mockAxios: AxiosInstance; - - beforeEach(() => { - vi.resetAllMocks(); - mockAxios = { - request: vi.fn(), - } as unknown as AxiosInstance; - }); - describe("Request Handling", () => { it("passes URL, signal, and responseType to axios", async () => { + const mockAxios = createAxiosMock(); const mockStream = createMockStream(); setupAxiosResponse(mockAxios, 200, {}, mockStream); @@ -37,7 +29,8 @@ describe("createStreamingFetchAdapter", () => { }); }); - it("applies headers in correct precedence order (config > init)", async () => { + it("applies headers in correct precedence order (config overrides init)", async () => { + const mockAxios = createAxiosMock(); const mockStream = createMockStream(); setupAxiosResponse(mockAxios, 200, {}, mockStream); @@ -88,6 +81,7 @@ describe("createStreamingFetchAdapter", () => { describe("Response Properties", () => { it("returns response with correct properties", async () => { + const mockAxios = createAxiosMock(); const mockStream = createMockStream(); setupAxiosResponse( mockAxios, @@ -102,11 +96,13 @@ describe("createStreamingFetchAdapter", () => { expect(response.url).toBe(TEST_URL); expect(response.status).toBe(200); expect(response.headers.get("content-type")).toBe("text/event-stream"); + // Headers are lowercased when we retrieve them expect(response.headers.get("CoNtEnT-TyPe")).toBe("text/event-stream"); expect(response.body?.getReader).toBeDefined(); }); it("detects redirected requests", async () => { + const mockAxios = createAxiosMock(); const mockStream = createMockStream(); const mockResponse = { status: 200, @@ -117,7 +113,7 @@ describe("createStreamingFetchAdapter", () => { responseUrl: "https://redirect.com/api", }, }, - }; + } as AxiosResponse; vi.mocked(mockAxios.request).mockResolvedValue(mockResponse); const adapter = createStreamingFetchAdapter(mockAxios); @@ -178,22 +174,14 @@ describe("createStreamingFetchAdapter", () => { expect(mockStream.destroy).toHaveBeenCalled(); }); }); - - async function setupReaderTest(): Promise<{ - mockStream: IncomingMessage; - reader: ReaderLike | ReadableStreamDefaultReader>; - }> { - const mockStream = createMockStream(); - setupAxiosResponse(mockAxios, 200, {}, mockStream); - - const adapter = createStreamingFetchAdapter(mockAxios); - const response = await adapter(TEST_URL); - const reader = response.body!.getReader(); - - return { mockStream, reader }; - } }); +function createAxiosMock(): AxiosInstance { + return { + request: vi.fn(), + } as unknown as AxiosInstance; +} + function createMockStream(): IncomingMessage { const stream = new EventEmitter() as IncomingMessage; stream.destroy = vi.fn(); @@ -212,3 +200,21 @@ function setupAxiosResponse( data: stream, }); } + +async function setupReaderTest(): Promise<{ + mockStream: IncomingMessage; + reader: ReaderLike | ReadableStreamDefaultReader>; +}> { + const mockAxios = createAxiosMock(); + const mockStream = createMockStream(); + setupAxiosResponse(mockAxios, 200, {}, mockStream); + + const adapter = createStreamingFetchAdapter(mockAxios); + const response = await adapter(TEST_URL); + const reader = response.body?.getReader(); + if (reader === undefined) { + throw new Error("Reader is undefined"); + } + + return { mockStream, reader }; +} diff --git a/test/unit/core/cliManager.test.ts b/test/unit/core/cliManager.test.ts index f2a2c2e5..d4f16c87 100644 --- a/test/unit/core/cliManager.test.ts +++ b/test/unit/core/cliManager.test.ts @@ -546,7 +546,8 @@ describe("CliManager", () => { expect(files.find((file) => file.includes(".asc"))).toBeUndefined(); }); - it.each([ + type SignatureErrorTestCase = [status: number, message: string]; + it.each([ [404, "Signature not found"], [500, "Failed to download signature"], ])("allows skipping verification on %i", async (status, message) => { @@ -558,7 +559,7 @@ describe("CliManager", () => { expect(pgp.verifySignature).not.toHaveBeenCalled(); }); - it.each([ + it.each([ [404, "Signature not found"], [500, "Failed to download signature"], ])( diff --git a/test/unit/logging/utils.test.ts b/test/unit/logging/utils.test.ts index 4d0f71eb..989a23e1 100644 --- a/test/unit/logging/utils.test.ts +++ b/test/unit/logging/utils.test.ts @@ -23,7 +23,8 @@ describe("Logging utils", () => { }); describe("sizeOf", () => { - it.each([ + type SizeOfTestCase = [data: unknown, bytes: number | undefined]; + it.each([ // Primitives return a fixed value [null, 0], [undefined, 0], diff --git a/test/unit/websocket/sseConnection.test.ts b/test/unit/websocket/sseConnection.test.ts index 9b2237bc..61cfce4d 100644 --- a/test/unit/websocket/sseConnection.test.ts +++ b/test/unit/websocket/sseConnection.test.ts @@ -1,9 +1,11 @@ import axios, { type AxiosInstance } from "axios"; import { type ServerSentEvent } from "coder/site/src/api/typesGenerated"; +import { type WebSocketEventType } from "coder/site/src/utils/OneWayWebSocket"; import { EventSource } from "eventsource"; -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { type CloseEvent, type ErrorEvent } from "ws"; +import { type Logger } from "@/logging/logger"; import { type ParsedMessageEvent } from "@/websocket/eventStreamConnection"; import { SseConnection } from "@/websocket/sseConnection"; @@ -20,17 +22,12 @@ vi.mock("@/api/streamingFetchAdapter", () => ({ })); describe("SseConnection", () => { - let mockAxios: AxiosInstance; - let mockLogger: ReturnType; - - beforeEach(() => { - vi.resetAllMocks(); - mockAxios = axios.create(); - mockLogger = createMockLogger(); - }); - describe("URL Building", () => { - it.each([ + type UrlBuildingTestCase = [ + searchParams: Record | URLSearchParams | undefined, + expectedUrl: string, + ]; + it.each([ [undefined, `${TEST_URL}${API_ROUTE}`], [ { follow: "true", after: "123" }, @@ -38,6 +35,8 @@ describe("SseConnection", () => { ], [new URLSearchParams({ foo: "bar" }), `${TEST_URL}${API_ROUTE}?foo=bar`], ])("constructs URL with %s search params", (searchParams, expectedUrl) => { + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); const mockES = createMockEventSource(); setupEventSourceMock(mockES); @@ -63,9 +62,11 @@ describe("SseConnection", () => { }); setupEventSourceMock(mockES); - const connection = createConnection(); - const events1: unknown[] = []; - const events2: unknown[] = []; + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); + const events1: object[] = []; + const events2: object[] = []; connection.addEventListener("open", (event) => events1.push(event)); connection.addEventListener("open", (event) => events2.push(event)); @@ -92,7 +93,9 @@ describe("SseConnection", () => { }); setupEventSourceMock(mockES); - const connection = createConnection(); + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); const events: ParsedMessageEvent[] = []; connection.addEventListener("message", (event) => events.push(event)); @@ -125,7 +128,9 @@ describe("SseConnection", () => { }); setupEventSourceMock(mockES); - const connection = createConnection(); + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); const events: ErrorEvent[] = []; connection.addEventListener("error", (event) => events.push(event)); @@ -143,9 +148,10 @@ describe("SseConnection", () => { addEventListener: vi.fn((event, handler) => { if (event === "error") { setImmediate(() => { + // A bit hacky but readyState is a readonly property so we have to override that here + const esWithReadyState = mockES as { readyState: number }; // Simulate EventSource behavior: state transitions to CLOSED when error occurs - (mockES as { readyState: number }).readyState = - EventSource.CLOSED; + esWithReadyState.readyState = EventSource.CLOSED; handler(new Event("error")); }); } @@ -153,7 +159,9 @@ describe("SseConnection", () => { }); setupEventSourceMock(mockES); - const connection = createConnection(); + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); const events: CloseEvent[] = []; connection.addEventListener("close", (event) => events.push(event)); @@ -170,18 +178,19 @@ describe("SseConnection", () => { describe("Event Listener Management", () => { it("removes event listener without affecting others", async () => { + const data = '{"test": true}'; const mockES = createMockEventSource({ addEventListener: vi.fn((event, handler) => { if (event === "data") { - setImmediate(() => - handler(new MessageEvent("data", { data: '{"test": true}' })), - ); + setImmediate(() => handler(new MessageEvent("data", { data }))); } }), }); setupEventSourceMock(mockES); - const connection = createConnection(); + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); const events: ParsedMessageEvent[] = []; const removedHandler = () => { @@ -195,13 +204,28 @@ describe("SseConnection", () => { connection.removeEventListener("message", removedHandler); await waitForNextTick(); - expect(events).toHaveLength(1); + // One message event + expect(events).toEqual([ + { + parseError: undefined, + parsedMessage: { + data: JSON.parse(data), + type: "data", + }, + sourceEvent: { data }, + }, + ]); expect(mockLogger.error).not.toHaveBeenCalled(); }); }); describe("Close Handling", () => { - it.each([ + type CloseHandlingTestCase = [ + code: number | undefined, + reason: string | undefined, + closeEvent: Omit, + ]; + it.each([ [ undefined, undefined, @@ -218,7 +242,9 @@ describe("SseConnection", () => { const mockES = createMockEventSource(); setupEventSourceMock(mockES); - const connection = createConnection(); + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); const events: CloseEvent[] = []; connection.addEventListener("close", (event) => events.push(event)); connection.addEventListener("open", () => {}); @@ -231,15 +257,20 @@ describe("SseConnection", () => { }); describe("Callback Error Handling", () => { - it.each([ + type CallbackErrorTestCase = [ + sseEvent: WebSocketEventType, + eventData: Event | MessageEvent, + ]; + it.each([ ["open", new Event("open")], ["message", new MessageEvent("data", { data: '{"test": true}' })], ["error", new Event("error")], - ] as const)( + ])( "logs error and continues when %s callback throws", async (sseEvent, eventData) => { const mockES = createMockEventSource({ addEventListener: vi.fn((event, handler) => { + // All SSE events are streaming data and attach a listener on the "data" type in the EventSource const esEvent = sseEvent === "message" ? "data" : sseEvent; if (event === esEvent) { setImmediate(() => handler(eventData)); @@ -248,7 +279,9 @@ describe("SseConnection", () => { }); setupEventSourceMock(mockES); - const connection = createConnection(); + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); const events: unknown[] = []; connection.addEventListener(sseEvent, () => { @@ -271,7 +304,9 @@ describe("SseConnection", () => { const mockES = createMockEventSource(); setupEventSourceMock(mockES); - const connection = createConnection(); + const mockAxios = axios.create(); + const mockLogger = createMockLogger(); + const connection = createConnection(mockAxios, mockLogger); connection.addEventListener("close", () => { throw new Error("Handler error"); }); @@ -285,17 +320,20 @@ describe("SseConnection", () => { ); }); }); - - function createConnection(): SseConnection { - return new SseConnection({ - location: { protocol: "https:", host: "coder.example.com" }, - apiRoute: API_ROUTE, - axiosInstance: mockAxios, - logger: mockLogger, - }); - } }); +function createConnection( + mockAxios: AxiosInstance, + mockLogger: Logger, +): SseConnection { + return new SseConnection({ + location: { protocol: "https:", host: "coder.example.com" }, + apiRoute: API_ROUTE, + axiosInstance: mockAxios, + logger: mockLogger, + }); +} + function createMockEventSource( overrides?: Partial, ): Partial { From d1cda296118ef87b45fb435b4eae437c4ae6c0ce Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 21 Oct 2025 12:17:04 +0300 Subject: [PATCH 3/3] Fix small sonar issues --- src/api/coderApi.ts | 11 ++++++----- src/api/streamingFetchAdapter.ts | 2 +- vitest.config.ts | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/api/coderApi.ts b/src/api/coderApi.ts index 6509ac67..da624bad 100644 --- a/src/api/coderApi.ts +++ b/src/api/coderApi.ts @@ -110,8 +110,9 @@ export class CoderApi extends Api { options?: ClientOptions, ) => { const searchParams = new URLSearchParams({ follow: "true" }); - if (logs.length) { - searchParams.append("after", logs[logs.length - 1].id.toString()); + const lastLog = logs.at(-1); + if (lastLog) { + searchParams.append("after", lastLog.id.toString()); } return this.createWebSocket({ @@ -311,9 +312,9 @@ function setupInterceptors( output, ); // Add headers from the header command. - Object.entries(headers).forEach(([key, value]) => { + for (const [key, value] of Object.entries(headers)) { config.headers[key] = value; - }); + } // Configure proxy and TLS. // Note that by default VS Code overrides the agent. To prevent this, set @@ -425,7 +426,7 @@ function wrapResponseTransform( function getSize(headers: AxiosHeaders, data: unknown): number | undefined { const contentLength = headers["content-length"]; if (contentLength !== undefined) { - return parseInt(contentLength, 10); + return Number.parseInt(contentLength, 10); } return sizeOf(data); diff --git a/src/api/streamingFetchAdapter.ts b/src/api/streamingFetchAdapter.ts index f0730535..f23ef1a7 100644 --- a/src/api/streamingFetchAdapter.ts +++ b/src/api/streamingFetchAdapter.ts @@ -1,6 +1,6 @@ import { type AxiosInstance } from "axios"; import { type FetchLikeInit, type FetchLikeResponse } from "eventsource"; -import { type IncomingMessage } from "http"; +import { type IncomingMessage } from "node:http"; /** * Creates a fetch adapter using an Axios instance that returns streaming responses. diff --git a/vitest.config.ts b/vitest.config.ts index 40c5f958..a3fcd089 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,4 +1,4 @@ -import path from "path"; +import path from "node:path"; import { defineConfig } from "vitest/config"; export default defineConfig({