Fix testing logic
This commit is contained in:
parent
c3cc678446
commit
82f11d8c86
3 changed files with 259 additions and 593 deletions
|
|
@ -9,6 +9,17 @@ import { Logger } from "../tracing/logger";
|
|||
import { assertSetContainsExactly } from "../utils/assert-set-contains-exactly";
|
||||
import type { FileSystemOperations } from "./filesystem-operations";
|
||||
import type { TextWithCursors } from "reconcile-text";
|
||||
import type { ServerConfig, ServerConfigData } from "../services/server-config";
|
||||
|
||||
class MockServerConfig implements Pick<ServerConfig, "getConfig"> {
|
||||
public getConfig(): ServerConfigData {
|
||||
return {
|
||||
mergeableFileExtensions: ["md", "txt"],
|
||||
supportedApiVersion: 1,
|
||||
isAuthenticated: true
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
class MockDatabase implements Partial<Database> {
|
||||
public getLatestDocumentByRelativePath(
|
||||
|
|
@ -79,7 +90,8 @@ describe("File operations", () => {
|
|||
const fileOperations = new FileOperations(
|
||||
new Logger(),
|
||||
new MockDatabase() as Database, // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
fileSystemOperations
|
||||
fileSystemOperations,
|
||||
new MockServerConfig() as ServerConfig // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
);
|
||||
|
||||
await fileOperations.create("a", new Uint8Array());
|
||||
|
|
@ -108,7 +120,8 @@ describe("File operations", () => {
|
|||
const fileOperations = new FileOperations(
|
||||
new Logger(),
|
||||
new MockDatabase() as Database, // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
fileSystemOperations
|
||||
fileSystemOperations,
|
||||
new MockServerConfig() as ServerConfig // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
);
|
||||
|
||||
await fileOperations.create("b.md", new Uint8Array());
|
||||
|
|
@ -147,7 +160,8 @@ describe("File operations", () => {
|
|||
const fileOperations = new FileOperations(
|
||||
new Logger(),
|
||||
new MockDatabase() as Database, // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
fileSystemOperations
|
||||
fileSystemOperations,
|
||||
new MockServerConfig() as ServerConfig // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
);
|
||||
|
||||
await fileOperations.create("a/b.c/d", new Uint8Array());
|
||||
|
|
@ -165,7 +179,8 @@ describe("File operations", () => {
|
|||
const fileOperations = new FileOperations(
|
||||
new Logger(),
|
||||
new MockDatabase() as Database, // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
fileSystemOperations
|
||||
fileSystemOperations,
|
||||
new MockServerConfig() as ServerConfig // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
);
|
||||
|
||||
await fileOperations.create("document (5).md", new Uint8Array());
|
||||
|
|
@ -193,7 +208,8 @@ describe("File operations", () => {
|
|||
const fileOperations = new FileOperations(
|
||||
new Logger(),
|
||||
new MockDatabase() as Database, // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
fileSystemOperations
|
||||
fileSystemOperations,
|
||||
new MockServerConfig() as ServerConfig // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
|
||||
);
|
||||
|
||||
await fileOperations.create(".gitignore", new Uint8Array());
|
||||
|
|
|
|||
|
|
@ -1,49 +1,62 @@
|
|||
/* eslint-disable @typescript-eslint/no-unsafe-type-assertion */
|
||||
import { describe, it, beforeEach } from "node:test";
|
||||
import assert from "node:assert";
|
||||
import { WebSocketManager } from "./websocket-manager";
|
||||
import type { Logger } from "../tracing/logger";
|
||||
import type { Settings } from "../persistence/settings";
|
||||
import type { WebSocketServerMessage } from "./types/WebSocketServerMessage";
|
||||
import type { WebSocketVaultUpdate } from "./types/WebSocketVaultUpdate";
|
||||
import type { ClientCursors } from "./types/ClientCursors";
|
||||
|
||||
class MockCloseEvent extends Event {
|
||||
public code: number;
|
||||
public reason: string;
|
||||
|
||||
public constructor(
|
||||
type: string,
|
||||
options: { code: number; reason: string }
|
||||
) {
|
||||
super(type);
|
||||
this.code = options.code;
|
||||
this.reason = options.reason;
|
||||
}
|
||||
}
|
||||
|
||||
class MockMessageEvent extends Event {
|
||||
public data: string;
|
||||
|
||||
public constructor(type: string, options: { data: string }) {
|
||||
super(type);
|
||||
this.data = options.data;
|
||||
}
|
||||
}
|
||||
|
||||
class MockWebSocket {
|
||||
public static readonly CONNECTING = 0;
|
||||
public static readonly OPEN = 1;
|
||||
public static readonly CLOSING = 2;
|
||||
public static readonly CLOSED = 3;
|
||||
|
||||
public readyState: number = MockWebSocket.CONNECTING;
|
||||
public readyState: number = WebSocket.CONNECTING;
|
||||
public onopen: ((event: Event) => void) | null = null;
|
||||
public onclose: ((event: CloseEvent) => void) | null = null;
|
||||
public onmessage: ((event: MessageEvent) => void) | null = null;
|
||||
public onclose: ((event: MockCloseEvent) => void) | null = null;
|
||||
public onmessage: ((event: MockMessageEvent) => void) | null = null;
|
||||
public onerror: ((event: Event) => void) | null = null;
|
||||
|
||||
public sentMessages: string[] = [];
|
||||
public closeCode: number | undefined;
|
||||
public closeReason: string | undefined;
|
||||
|
||||
public constructor(public url: string) {
|
||||
// Simulate async connection
|
||||
setTimeout(() => {
|
||||
if (this.readyState === MockWebSocket.CONNECTING) {
|
||||
this.readyState = MockWebSocket.OPEN;
|
||||
if (this.readyState === WebSocket.CONNECTING) {
|
||||
this.readyState = WebSocket.OPEN;
|
||||
this.onopen?.(new Event("open"));
|
||||
}
|
||||
}, 0);
|
||||
}
|
||||
|
||||
public send(data: string): void {
|
||||
if (this.readyState !== MockWebSocket.OPEN) {
|
||||
if (this.readyState !== WebSocket.OPEN) {
|
||||
throw new Error("WebSocket is not open");
|
||||
}
|
||||
this.sentMessages.push(data);
|
||||
}
|
||||
|
||||
public close(code?: number, reason?: string): void {
|
||||
this.closeCode = code;
|
||||
this.closeReason = reason;
|
||||
this.readyState = MockWebSocket.CLOSED;
|
||||
this.readyState = WebSocket.CLOSED;
|
||||
this.onclose?.(
|
||||
new CloseEvent("close", {
|
||||
new MockCloseEvent("close", {
|
||||
code: code ?? 1000,
|
||||
reason: reason ?? ""
|
||||
})
|
||||
|
|
@ -52,27 +65,46 @@ class MockWebSocket {
|
|||
|
||||
public simulateMessage(data: unknown): void {
|
||||
this.onmessage?.(
|
||||
new MessageEvent("message", { data: JSON.stringify(data) })
|
||||
new MockMessageEvent("message", { data: JSON.stringify(data) })
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
type MockFn<T extends (...args: unknown[]) => unknown> = T & {
|
||||
calls: Parameters<T>[];
|
||||
};
|
||||
|
||||
function createMockFn<T extends (...args: unknown[]) => unknown>(
|
||||
implementation?: T
|
||||
): MockFn<T> {
|
||||
const calls: Parameters<T>[] = [];
|
||||
const mockFn = ((...args: Parameters<T>) => {
|
||||
calls.push(args);
|
||||
return implementation?.(...args);
|
||||
}) as unknown as MockFn<T>;
|
||||
mockFn.calls = calls;
|
||||
return mockFn;
|
||||
}
|
||||
|
||||
describe("WebSocketManager", () => {
|
||||
let mockLogger: Logger;
|
||||
let mockSettings: Settings;
|
||||
let deviceId: string;
|
||||
let mockLogger: Logger = undefined as unknown as Logger;
|
||||
let mockSettings: Settings = undefined as unknown as Settings;
|
||||
let deviceId = "test-device-123";
|
||||
|
||||
beforeEach(() => {
|
||||
deviceId = "test-device-123";
|
||||
const noop = (): void => {
|
||||
// Intentionally empty for mock
|
||||
};
|
||||
mockLogger = {
|
||||
info: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
debug: jest.fn()
|
||||
info: createMockFn(noop),
|
||||
warn: createMockFn(noop),
|
||||
error: createMockFn(noop),
|
||||
debug: createMockFn(noop)
|
||||
} as unknown as Logger;
|
||||
|
||||
mockSettings = {
|
||||
getSettings: jest.fn().mockReturnValue({
|
||||
getSettings: () => ({
|
||||
remoteUri: "https://example.com",
|
||||
vaultName: "test-vault",
|
||||
webSocketRetryIntervalMs: 1000
|
||||
|
|
@ -80,62 +112,7 @@ describe("WebSocketManager", () => {
|
|||
} as unknown as Settings;
|
||||
});
|
||||
|
||||
describe("BUG #1: Promise Tracking Memory Leak", () => {
|
||||
it("EXPOSES: promises are never removed from outstandingPromises array", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
let listenerCallCount = 0;
|
||||
const listener = jest.fn(async () => {
|
||||
listenerCallCount++;
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
});
|
||||
|
||||
manager.addRemoteVaultUpdateListener(listener);
|
||||
manager.start();
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const vaultUpdate: WebSocketServerMessage = {
|
||||
type: "vaultUpdate",
|
||||
updates: []
|
||||
};
|
||||
|
||||
// Access private field to inspect outstandingPromises
|
||||
const outstandingPromises = (manager as unknown as {
|
||||
outstandingPromises: Promise<unknown>[];
|
||||
}).outstandingPromises;
|
||||
|
||||
// Send multiple messages
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
mockWs.simulateMessage(vaultUpdate);
|
||||
mockWs.simulateMessage(vaultUpdate);
|
||||
mockWs.simulateMessage(vaultUpdate);
|
||||
|
||||
// Wait for listeners to complete
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
// BUG: The promises should have been removed after completion,
|
||||
// but due to the tracking bug, they accumulate in the array
|
||||
// The finally() handler tries to remove `trackedPromise` but
|
||||
// outstandingPromises contains the wrapper promises
|
||||
expect(outstandingPromises.length).toBeGreaterThan(0);
|
||||
expect(listenerCallCount).toBe(3);
|
||||
|
||||
// This demonstrates the memory leak - promises never get cleaned up
|
||||
console.log(
|
||||
`MEMORY LEAK: ${outstandingPromises.length} promises still tracked after completion`
|
||||
);
|
||||
|
||||
await manager.stop();
|
||||
});
|
||||
|
||||
it("EXPOSES: promises accumulate over many messages", async () => {
|
||||
it("cleans up promises after message handling", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
|
|
@ -144,39 +121,28 @@ describe("WebSocketManager", () => {
|
|||
);
|
||||
|
||||
manager.addRemoteVaultUpdateListener(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 5));
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
});
|
||||
manager.start();
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const outstandingPromises = (manager as unknown as {
|
||||
const { outstandingPromises } = manager as unknown as {
|
||||
outstandingPromises: Promise<unknown>[];
|
||||
}).outstandingPromises;
|
||||
|
||||
};
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
// Send 10 messages
|
||||
for (let i = 0; i < 10; i++) {
|
||||
mockWs.simulateMessage({
|
||||
type: "vaultUpdate",
|
||||
updates: []
|
||||
});
|
||||
}
|
||||
mockWs.simulateMessage({ type: "vaultUpdate", updates: [] });
|
||||
mockWs.simulateMessage({ type: "vaultUpdate", updates: [] });
|
||||
mockWs.simulateMessage({ type: "vaultUpdate", updates: [] });
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
// BUG: All 10 promises should be cleaned up, but they're not
|
||||
expect(outstandingPromises.length).toBe(10);
|
||||
console.log(
|
||||
`MEMORY LEAK: ${outstandingPromises.length} promises accumulated`
|
||||
);
|
||||
|
||||
assert.strictEqual(outstandingPromises.length, 0);
|
||||
await manager.stop();
|
||||
});
|
||||
|
||||
it("EXPOSES: same bug occurs with cursor position messages", async () => {
|
||||
it("cleans up cursor position promises", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
|
|
@ -188,92 +154,25 @@ describe("WebSocketManager", () => {
|
|||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
});
|
||||
manager.start();
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const outstandingPromises = (manager as unknown as {
|
||||
const { outstandingPromises } = manager as unknown as {
|
||||
outstandingPromises: Promise<unknown>[];
|
||||
}).outstandingPromises;
|
||||
|
||||
};
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
const cursorMessage: WebSocketServerMessage = {
|
||||
mockWs.simulateMessage({
|
||||
type: "cursorPositions",
|
||||
clients: [
|
||||
{
|
||||
deviceId: "other-device",
|
||||
cursors: []
|
||||
}
|
||||
]
|
||||
};
|
||||
|
||||
mockWs.simulateMessage(cursorMessage);
|
||||
mockWs.simulateMessage(cursorMessage);
|
||||
clients: [{ deviceId: "other-device", cursors: [] }]
|
||||
});
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
// BUG: Same promise tracking bug affects cursor messages
|
||||
expect(outstandingPromises.length).toBe(2);
|
||||
|
||||
assert.strictEqual(outstandingPromises.length, 0);
|
||||
await manager.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("BUG #2: Redundant WebSocket Checks", () => {
|
||||
it("EXPOSES: updateLocalCursors logs duplicate warnings", () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
// Don't start, so WebSocket is not connected
|
||||
manager.updateLocalCursors({ cursors: [] });
|
||||
|
||||
// BUG: Two warning logs are generated for the same condition
|
||||
expect(mockLogger.warn).toHaveBeenCalledTimes(2);
|
||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||
"WebSocket is not connected, cannot send cursor positions"
|
||||
);
|
||||
});
|
||||
|
||||
it("EXPOSES: race condition between checks", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
// Manually set WebSocket to closing state after first check
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
const originalReadyState = mockWs.readyState;
|
||||
|
||||
// Simulate race condition: connection drops between the two checks
|
||||
jest.spyOn(mockWs, "readyState", "get")
|
||||
.mockReturnValueOnce(MockWebSocket.OPEN) // First check passes
|
||||
.mockReturnValueOnce(MockWebSocket.CLOSED); // Second check fails
|
||||
|
||||
manager.updateLocalCursors({ cursors: [] });
|
||||
|
||||
// BUG: Even though first check passed, second check fails
|
||||
// This demonstrates the race condition
|
||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||
"WebSocket is not connected, cannot send cursor positions"
|
||||
);
|
||||
|
||||
await manager.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("BUG #3: Missing Error Handling on send()", () => {
|
||||
it("EXPOSES: sendHandshakeMessage crashes when send() throws", async () => {
|
||||
it("logs handshake send errors", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
|
|
@ -286,26 +185,23 @@ describe("WebSocketManager", () => {
|
|||
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
// Simulate send() throwing an error (e.g., buffer full)
|
||||
jest.spyOn(mockWs, "send").mockImplementation(() => {
|
||||
mockWs.send = (): void => {
|
||||
throw new Error("Buffer full");
|
||||
});
|
||||
};
|
||||
|
||||
// BUG: This throws and crashes - no try-catch to handle it
|
||||
expect(() => {
|
||||
assert.throws(() => {
|
||||
manager.sendHandshakeMessage({
|
||||
type: "handshake",
|
||||
vaultName: "test",
|
||||
token: "test",
|
||||
deviceId: "test",
|
||||
authToken: "test"
|
||||
lastSeenVaultUpdateId: null
|
||||
});
|
||||
});
|
||||
}).toThrow("Buffer full");
|
||||
|
||||
await manager.stop();
|
||||
});
|
||||
|
||||
it("EXPOSES: updateLocalCursors crashes when send() throws", async () => {
|
||||
it("completes stop with timeout protection", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
|
|
@ -316,22 +212,11 @@ describe("WebSocketManager", () => {
|
|||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
jest.spyOn(mockWs, "send").mockImplementation(() => {
|
||||
throw new Error("Connection closed");
|
||||
});
|
||||
|
||||
// BUG: This throws and crashes - no try-catch to handle it
|
||||
expect(() => {
|
||||
manager.updateLocalCursors({ cursors: [] });
|
||||
}).toThrow("Connection closed");
|
||||
|
||||
await manager.stop();
|
||||
assert.ok(true);
|
||||
});
|
||||
|
||||
it("EXPOSES: send() can throw even after isWebSocketConnected check", async () => {
|
||||
it("clears old handlers on reconnection", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
|
|
@ -339,147 +224,7 @@ describe("WebSocketManager", () => {
|
|||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
// WebSocket is open, but send fails
|
||||
expect(manager.isWebSocketConnected).toBe(true);
|
||||
|
||||
jest.spyOn(mockWs, "send").mockImplementation(() => {
|
||||
throw new Error("Unexpected error");
|
||||
});
|
||||
|
||||
// BUG: Even though connection check passed, send() can still throw
|
||||
expect(() => {
|
||||
manager.updateLocalCursors({ cursors: [] });
|
||||
}).toThrow("Unexpected error");
|
||||
|
||||
await manager.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("BUG #4: Potential Infinite Loop in stop()", () => {
|
||||
it("EXPOSES: stop() hangs if onclose handler never fires", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
// Simulate a broken WebSocket that doesn't fire onclose
|
||||
jest.spyOn(mockWs, "close").mockImplementation(() => {
|
||||
// Close is called but onclose handler is never invoked
|
||||
mockWs.readyState = MockWebSocket.CLOSING; // Stuck in CLOSING
|
||||
// Don't call onclose
|
||||
});
|
||||
|
||||
// BUG: This will hang forever because the while loop waits for
|
||||
// isWebSocketConnected to become false, but it never does
|
||||
const stopPromise = manager.stop();
|
||||
|
||||
// Wait a bit to show it's stuck
|
||||
const timeoutPromise = new Promise((resolve) =>
|
||||
setTimeout(() => resolve("timeout"), 100)
|
||||
);
|
||||
|
||||
const result = await Promise.race([stopPromise, timeoutPromise]);
|
||||
|
||||
expect(result).toBe("timeout");
|
||||
console.log("BUG: stop() is stuck in infinite loop");
|
||||
|
||||
// Note: We can't actually clean up here because stop() is hung
|
||||
// In a real scenario, this would freeze the application
|
||||
});
|
||||
|
||||
it("EXPOSES: stop() loops forever if WebSocket state is corrupted", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
// Corrupt the WebSocket state
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
jest.spyOn(mockWs, "close").mockImplementation(() => {
|
||||
// Intentionally leave readyState as OPEN
|
||||
// This simulates a bug or corrupted state
|
||||
});
|
||||
|
||||
const stopPromise = manager.stop();
|
||||
const timeoutPromise = new Promise((resolve) =>
|
||||
setTimeout(() => resolve("timeout"), 100)
|
||||
);
|
||||
|
||||
const result = await Promise.race([stopPromise, timeoutPromise]);
|
||||
|
||||
// BUG: Infinite loop because readyState never changes
|
||||
expect(result).toBe("timeout");
|
||||
});
|
||||
});
|
||||
|
||||
describe("BUG #5: WebSocket Handler Race Condition", () => {
|
||||
it("EXPOSES: rapid reconnection creates multiple WebSocket instances", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const firstWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
// Trigger reconnection by calling initializeWebSocket again
|
||||
(manager as unknown as { initializeWebSocket: () => void })
|
||||
.initializeWebSocket();
|
||||
|
||||
const secondWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
// BUG: Two different WebSocket instances exist
|
||||
expect(firstWs).not.toBe(secondWs);
|
||||
|
||||
// The old WebSocket's handlers are still registered and can fire
|
||||
// This can cause interference and unexpected behavior
|
||||
|
||||
// Simulate the old WebSocket's onclose firing
|
||||
firstWs.onclose?.(
|
||||
new CloseEvent("close", { code: 1000, reason: "test" })
|
||||
);
|
||||
|
||||
// This could trigger reconnection logic even though we have a new WebSocket
|
||||
// The status change listeners will be called multiple times
|
||||
|
||||
await manager.stop();
|
||||
});
|
||||
|
||||
it("EXPOSES: old WebSocket handlers interfere with new connection", async () => {
|
||||
let statusChangeCount = 0;
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
manager.addWebSocketStatusChangeListener(() => {
|
||||
statusChangeCount++;
|
||||
});
|
||||
|
|
@ -490,29 +235,25 @@ describe("WebSocketManager", () => {
|
|||
const firstWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
|
||||
// Reset counter after initial connection
|
||||
statusChangeCount = 0;
|
||||
|
||||
// Create new WebSocket
|
||||
(manager as unknown as { initializeWebSocket: () => void })
|
||||
.initializeWebSocket();
|
||||
(
|
||||
manager as unknown as { initializeWebSocket: () => void }
|
||||
).initializeWebSocket();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
// Now trigger old WebSocket's onclose
|
||||
statusChangeCount = 0;
|
||||
|
||||
// Old handler should be cleared
|
||||
firstWs.onclose?.(
|
||||
new CloseEvent("close", { code: 1000, reason: "test" })
|
||||
new MockCloseEvent("close", { code: 1000, reason: "test" })
|
||||
);
|
||||
|
||||
// BUG: Status change listeners are called for old connection
|
||||
// This can cause confusion and incorrect state
|
||||
expect(statusChangeCount).toBeGreaterThan(0);
|
||||
|
||||
assert.strictEqual(statusChangeCount, 0);
|
||||
await manager.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("BUG #6: Untracked handleWebSocketMessage Promise", () => {
|
||||
it("EXPOSES: handleWebSocketMessage promise not in outstandingPromises", async () => {
|
||||
it("tracks message handling promises", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
|
|
@ -520,6 +261,7 @@ describe("WebSocketManager", () => {
|
|||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/init-declarations
|
||||
let resolveListener: () => void;
|
||||
const listenerPromise = new Promise<void>((resolve) => {
|
||||
resolveListener = resolve;
|
||||
|
|
@ -534,113 +276,21 @@ describe("WebSocketManager", () => {
|
|||
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
mockWs.simulateMessage({ type: "vaultUpdate", updates: [] });
|
||||
|
||||
// Send message - this triggers handleWebSocketMessage
|
||||
mockWs.simulateMessage({
|
||||
type: "vaultUpdate",
|
||||
updates: []
|
||||
});
|
||||
|
||||
// Give time for handleWebSocketMessage to start
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
|
||||
// Now try to stop - the handleWebSocketMessage promise is still running
|
||||
const stopPromise = manager.stop();
|
||||
const { outstandingPromises } = manager as unknown as {
|
||||
outstandingPromises: Promise<unknown>[];
|
||||
};
|
||||
|
||||
// BUG: stop() awaits outstandingPromises, but handleWebSocketMessage
|
||||
// itself is not tracked, only the listener promises inside it are
|
||||
// However, due to bug #1, even those aren't properly tracked
|
||||
assert.ok(outstandingPromises.length > 0);
|
||||
|
||||
// Resolve the listener to allow stop to complete
|
||||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
|
||||
resolveListener!();
|
||||
|
||||
await stopPromise;
|
||||
|
||||
// This test demonstrates that the outer handleWebSocketMessage
|
||||
// promise is not being tracked
|
||||
});
|
||||
});
|
||||
|
||||
describe("Additional Edge Cases", () => {
|
||||
it("multiple listeners with different completion times", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
const listener1 = jest.fn(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
});
|
||||
const listener2 = jest.fn(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
});
|
||||
const listener3 = jest.fn(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 5));
|
||||
});
|
||||
|
||||
manager.addRemoteVaultUpdateListener(listener1);
|
||||
manager.addRemoteVaultUpdateListener(listener2);
|
||||
manager.addRemoteVaultUpdateListener(listener3);
|
||||
|
||||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const outstandingPromises = (manager as unknown as {
|
||||
outstandingPromises: Promise<unknown>[];
|
||||
}).outstandingPromises;
|
||||
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
mockWs.simulateMessage({ type: "vaultUpdate", updates: [] });
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
// BUG: Even though all listeners completed, 3 promises remain
|
||||
expect(outstandingPromises.length).toBe(3);
|
||||
expect(listener1).toHaveBeenCalledTimes(1);
|
||||
expect(listener2).toHaveBeenCalledTimes(1);
|
||||
expect(listener3).toHaveBeenCalledTimes(1);
|
||||
|
||||
assert.strictEqual(outstandingPromises.length, 0);
|
||||
await manager.stop();
|
||||
});
|
||||
|
||||
it("listener throws error - promise still not cleaned up", async () => {
|
||||
const manager = new WebSocketManager(
|
||||
deviceId,
|
||||
mockLogger,
|
||||
mockSettings,
|
||||
MockWebSocket as unknown as typeof WebSocket
|
||||
);
|
||||
|
||||
const errorListener = jest.fn(async () => {
|
||||
throw new Error("Listener error");
|
||||
});
|
||||
|
||||
manager.addRemoteVaultUpdateListener(errorListener);
|
||||
manager.start();
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
const outstandingPromises = (manager as unknown as {
|
||||
outstandingPromises: Promise<unknown>[];
|
||||
}).outstandingPromises;
|
||||
|
||||
const mockWs = (manager as unknown as { webSocket: MockWebSocket })
|
||||
.webSocket;
|
||||
mockWs.simulateMessage({ type: "vaultUpdate", updates: [] });
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
// Error should be logged
|
||||
expect(mockLogger.error).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Error in vault update listener")
|
||||
);
|
||||
|
||||
// BUG: Promise still not removed even after error
|
||||
expect(outstandingPromises.length).toBe(1);
|
||||
|
||||
await manager.stop();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,17 +13,17 @@ export function slowWebSocketFactory(
|
|||
|
||||
private readonly locks = new Locks(logger);
|
||||
|
||||
public set onopen(callback: (event: Event) => void) {
|
||||
public set onopen(callback: ((event: Event) => void) | null) {
|
||||
super.onopen = async (event: Event): Promise<void> => {
|
||||
if (jitterScaleInSeconds > 0) {
|
||||
await sleep(Math.random() * jitterScaleInSeconds * 1000);
|
||||
}
|
||||
|
||||
callback(event);
|
||||
callback?.(event);
|
||||
};
|
||||
}
|
||||
|
||||
public set onmessage(callback: (event: MessageEvent) => void) {
|
||||
public set onmessage(callback: ((event: MessageEvent) => void) | null) {
|
||||
super.onmessage = async (event: MessageEvent): Promise<void> => {
|
||||
await this.locks.withLock(
|
||||
FlakyWebSocket.RECEIVE_KEY,
|
||||
|
|
@ -34,27 +34,27 @@ export function slowWebSocketFactory(
|
|||
);
|
||||
}
|
||||
|
||||
callback(event);
|
||||
callback?.(event);
|
||||
}
|
||||
);
|
||||
};
|
||||
}
|
||||
|
||||
public set onclose(callback: (event: CloseEvent) => void) {
|
||||
public set onclose(callback: ((event: CloseEvent) => void) | null) {
|
||||
super.onclose = async (event: CloseEvent): Promise<void> => {
|
||||
if (jitterScaleInSeconds > 0) {
|
||||
await sleep(Math.random() * jitterScaleInSeconds * 1000);
|
||||
}
|
||||
callback(event);
|
||||
callback?.(event);
|
||||
};
|
||||
}
|
||||
|
||||
public set onerror(callback: (event: Event) => void) {
|
||||
public set onerror(callback: ((event: Event) => void) | null) {
|
||||
super.onerror = async (event: Event): Promise<void> => {
|
||||
if (jitterScaleInSeconds > 0) {
|
||||
await sleep(Math.random() * jitterScaleInSeconds * 1000);
|
||||
}
|
||||
callback(event);
|
||||
callback?.(event);
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue