From 2b7efc884491f70bfb29fad38ff05de36efa7327 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 17 Dec 2025 16:37:19 +0100 Subject: [PATCH 1/4] feat: Add timeout handling for remote message sends and URL redemptions --- .../src/remotes/RemoteHandle.test.ts | 118 +++++++++++++++++- .../ocap-kernel/src/remotes/RemoteHandle.ts | 31 ++++- .../ocap-kernel/src/remotes/network.test.ts | 1 + packages/ocap-kernel/src/remotes/network.ts | 29 ++++- .../repo-tools/src/test-utils/abort-signal.ts | 51 ++++++++ packages/repo-tools/src/test-utils/index.ts | 1 + vitest.config.ts | 2 +- 7 files changed, 224 insertions(+), 9 deletions(-) create mode 100644 packages/repo-tools/src/test-utils/abort-signal.ts diff --git a/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts b/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts index f04c9c863..fd3b7ae5d 100644 --- a/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts +++ b/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts @@ -1,6 +1,7 @@ import type { VatOneResolution } from '@agoric/swingset-liveslots'; import type { Logger } from '@metamask/logger'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { makeAbortSignalMock } from '@ocap/repo-tools/test-utils'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { KernelQueue } from '../KernelQueue.ts'; import { RemoteHandle } from './RemoteHandle.ts'; @@ -626,4 +627,119 @@ describe('RemoteHandle', () => { // Verify they resolved independently (different values) expect(kref1).not.toBe(kref2); }); + + describe('redeemOcapURL timeout', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('sets up 30-second timeout using AbortSignal.timeout', async () => { + const remote = makeRemote(); + const mockOcapURL = 'ocap:test@peer'; + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + const urlPromise = remote.redeemOcapURL(mockOcapURL); + + // Verify AbortSignal.timeout was called with 30 seconds + expect(AbortSignal.timeout).toHaveBeenCalledWith(30_000); + expect(mockSignal?.timeoutMs).toBe(30_000); + + // Resolve the redemption to avoid hanging + const sendCall = vi.mocked(mockRemoteComms.sendRemoteMessage).mock + .calls[0]; + const sentMessage = JSON.parse(sendCall?.[1] as string); + const replyKey = sentMessage.params[1] as string; + + await remote.handleRemoteMessage( + JSON.stringify({ + method: 'redeemURLReply', + params: [true, replyKey, 'ro+1'], + }), + ); + + await urlPromise; + }); + + it('cleans up pending redemption when redemption succeeds before timeout', async () => { + const remote = makeRemote(); + const mockOcapURL = 'ocap:test@peer'; + const mockURLResolutionRRef = 'ro+6'; + const mockURLResolutionKRef = 'ko1'; + const expectedReplyKey = '1'; + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + const urlPromise = remote.redeemOcapURL(mockOcapURL); + + // Send reply immediately (before timeout) + const redeemURLReply = { + method: 'redeemURLReply', + params: [true, expectedReplyKey, mockURLResolutionRRef], + }; + await remote.handleRemoteMessage(JSON.stringify(redeemURLReply)); + + const kref = await urlPromise; + expect(kref).toBe(mockURLResolutionKRef); + + // Verify timeout signal was not aborted + expect(mockSignal?.aborted).toBe(false); + + // Verify cleanup happened - trying to handle another reply with the same key should fail + await expect( + remote.handleRemoteMessage(JSON.stringify(redeemURLReply)), + ).rejects.toThrow(`unknown URL redemption reply key ${expectedReplyKey}`); + }); + + it('cleans up pending redemption map entry on timeout', async () => { + const remote = makeRemote(); + const mockOcapURL = 'ocap:test@peer'; + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + // Start a redemption + const urlPromise = remote.redeemOcapURL(mockOcapURL); + + // Get the reply key that was used + const sendCall = vi.mocked(mockRemoteComms.sendRemoteMessage).mock + .calls[0]; + const sentMessage = JSON.parse(sendCall?.[1] as string); + const replyKey = sentMessage.params[1] as string; + + // Wait for the promise to be set up and event listener registered + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Manually trigger the abort to simulate timeout + mockSignal?.abort(); + + // Wait for the abort handler to execute + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Verify the promise rejects + await expect(urlPromise).rejects.toThrow( + 'URL redemption timed out after 30 seconds', + ); + + // Verify cleanup happened - trying to handle a reply with the same key should fail + const redeemURLReply = { + method: 'redeemURLReply', + params: [true, replyKey, 'ro+1'], + }; + await expect( + remote.handleRemoteMessage(JSON.stringify(redeemURLReply)), + ).rejects.toThrow(`unknown URL redemption reply key ${replyKey}`); + }); + }); }); diff --git a/packages/ocap-kernel/src/remotes/RemoteHandle.ts b/packages/ocap-kernel/src/remotes/RemoteHandle.ts index 1510e3ecf..5bba75562 100644 --- a/packages/ocap-kernel/src/remotes/RemoteHandle.ts +++ b/packages/ocap-kernel/src/remotes/RemoteHandle.ts @@ -460,13 +460,34 @@ export class RemoteHandle implements EndpointHandle { const replyKey = `${this.#redemptionCounter}`; this.#redemptionCounter += 1; const { promise, resolve, reject } = makePromiseKit(); - // XXX TODO: Probably these should have timeouts this.#pendingRedemptions.set(replyKey, [resolve, reject]); - await this.#sendRemoteCommand({ - method: 'redeemURL', - params: [url, replyKey], + + // Set up timeout handling with AbortSignal + const timeoutSignal = AbortSignal.timeout(30_000); + const timeoutPromise = new Promise((_resolve, _reject) => { + timeoutSignal.addEventListener('abort', () => { + // Clean up from pending redemptions map + if (this.#pendingRedemptions.has(replyKey)) { + this.#pendingRedemptions.delete(replyKey); + } + _reject(new Error('URL redemption timed out after 30 seconds')); + }); }); - return promise; + + try { + await this.#sendRemoteCommand({ + method: 'redeemURL', + params: [url, replyKey], + }); + // Wait for reply with timeout protection + return await Promise.race([promise, timeoutPromise]); + } catch (error) { + // Clean up and remove from map if still pending + if (this.#pendingRedemptions.has(replyKey)) { + this.#pendingRedemptions.delete(replyKey); + } + throw error; + } } /** diff --git a/packages/ocap-kernel/src/remotes/network.test.ts b/packages/ocap-kernel/src/remotes/network.test.ts index 8f5bf744a..c418b4eeb 100644 --- a/packages/ocap-kernel/src/remotes/network.test.ts +++ b/packages/ocap-kernel/src/remotes/network.test.ts @@ -1,4 +1,5 @@ import { AbortError } from '@metamask/kernel-errors'; +import { makeAbortSignalMock } from '@ocap/repo-tools/test-utils'; import { describe, expect, diff --git a/packages/ocap-kernel/src/remotes/network.ts b/packages/ocap-kernel/src/remotes/network.ts index 55943da94..060be1a3d 100644 --- a/packages/ocap-kernel/src/remotes/network.ts +++ b/packages/ocap-kernel/src/remotes/network.ts @@ -100,6 +100,31 @@ export async function initNetwork( return queue; } + /** + * Write a message to a channel stream with a timeout. + * + * @param channel - The channel to write to. + * @param message - The message bytes to write. + * @param timeoutMs - Timeout in milliseconds (default: 10 seconds). + * @returns Promise that resolves when the write completes or rejects on timeout. + * @throws Error if the write times out or fails. + */ + async function writeWithTimeout( + channel: Channel, + message: Uint8Array, + timeoutMs = 10_000, + ): Promise { + const timeoutSignal = AbortSignal.timeout(timeoutMs); + return Promise.race([ + channel.msgStream.write(message), + new Promise((_resolve, reject) => { + timeoutSignal.addEventListener('abort', () => { + reject(new Error(`Message send timed out after ${timeoutMs}ms`)); + }); + }), + ]); + } + /** * Receive a message from a peer. * @@ -315,7 +340,7 @@ export async function initNetwork( while ((queuedMsg = queue.dequeue()) !== undefined) { try { logger.log(`${peerId}:: send (queued) ${queuedMsg.message}`); - await channel.msgStream.write(fromString(queuedMsg.message)); + await writeWithTimeout(channel, fromString(queuedMsg.message), 10_000); } catch (problem) { outputError(peerId, `sending queued message`, problem); // Preserve the failed message and all remaining messages @@ -397,7 +422,7 @@ export async function initNetwork( try { logger.log(`${targetPeerId}:: send ${message}`); - await channel.msgStream.write(fromString(message)); + await writeWithTimeout(channel, fromString(message), 10_000); reconnectionManager.resetBackoff(targetPeerId); } catch (problem) { outputError(targetPeerId, `sending message`, problem); diff --git a/packages/repo-tools/src/test-utils/abort-signal.ts b/packages/repo-tools/src/test-utils/abort-signal.ts new file mode 100644 index 000000000..4dbbc3240 --- /dev/null +++ b/packages/repo-tools/src/test-utils/abort-signal.ts @@ -0,0 +1,51 @@ +import { vi } from 'vitest'; + +/** + * Create a mock AbortSignal that can be manually aborted. + * + * @param timeoutMs - The timeout value (stored for verification). + * @returns A mock AbortSignal. + */ +export function makeAbortSignalMock(timeoutMs: number): AbortSignal & { + abort: () => void; + timeoutMs: number; +} { + const handlers: (() => void)[] = []; + let aborted = false; + + const signal = { + get aborted() { + return aborted; + }, + timeoutMs, + addEventListener: vi.fn((event: string, handler: () => void) => { + if (event === 'abort') { + handlers.push(handler); + } + }), + removeEventListener: vi.fn((event: string, handler: () => void) => { + if (event === 'abort') { + const index = handlers.indexOf(handler); + if (index > -1) { + handlers.splice(index, 1); + } + } + }), + dispatchEvent: vi.fn(), + onabort: null, + reason: undefined, + throwIfAborted: vi.fn(), + abort() { + aborted = true; + // Call all handlers synchronously + for (const handler of handlers) { + handler(); + } + }, + } as AbortSignal & { + abort: () => void; + timeoutMs: number; + }; + + return signal; +} diff --git a/packages/repo-tools/src/test-utils/index.ts b/packages/repo-tools/src/test-utils/index.ts index 97438a3eb..1739508b2 100644 --- a/packages/repo-tools/src/test-utils/index.ts +++ b/packages/repo-tools/src/test-utils/index.ts @@ -4,6 +4,7 @@ export { makePromiseKitMock } from './promise-kit.ts'; export { fetchMock } from './env/fetch-mock.ts'; export * from './env/mock-kernel.ts'; export { makeMockMessageTarget } from './postMessage.ts'; +export { makeAbortSignalMock } from './abort-signal.ts'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore Intermittent browser/Node incompatibility diff --git a/vitest.config.ts b/vitest.config.ts index cf60c78e4..c5d7b62b6 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -106,7 +106,7 @@ export default defineConfig({ 'packages/kernel-platforms/**': { statements: 99.38, functions: 100, - branches: 96.2, + branches: 96.25, lines: 99.38, }, 'packages/kernel-rpc-methods/**': { From f55197e89a8692f5a0f53e7f06f7bd0a56111122 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 17 Dec 2025 16:42:42 +0100 Subject: [PATCH 2/4] add mock note --- packages/repo-tools/src/test-utils/abort-signal.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/repo-tools/src/test-utils/abort-signal.ts b/packages/repo-tools/src/test-utils/abort-signal.ts index 4dbbc3240..0d62c3a17 100644 --- a/packages/repo-tools/src/test-utils/abort-signal.ts +++ b/packages/repo-tools/src/test-utils/abort-signal.ts @@ -3,6 +3,13 @@ import { vi } from 'vitest'; /** * Create a mock AbortSignal that can be manually aborted. * + * This utility was created because Vitest cannot mock `AbortSignal.timeout()`. + * Vitest relies on @sinonjs/fake-timers for timer mocking, but fake-timers does + * not implement the AbortSignal.timeout API, so we cannot use Vitest's timer + * mocking to test timeout behavior. This mock allows us to manually trigger + * abort events in tests to simulate timeout scenarios. + * https://github.com/vitest-dev/vitest/issues/3088 + * * @param timeoutMs - The timeout value (stored for verification). * @returns A mock AbortSignal. */ From 84e73459b960802a0e128e7131ed6bff54a23114 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 17 Dec 2025 16:53:01 +0100 Subject: [PATCH 3/4] fix the unhandled promise rejection bug --- .../ocap-kernel/src/remotes/RemoteHandle.ts | 12 ++++++-- packages/ocap-kernel/src/remotes/network.ts | 28 +++++++++++++------ vitest.config.ts | 2 +- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/ocap-kernel/src/remotes/RemoteHandle.ts b/packages/ocap-kernel/src/remotes/RemoteHandle.ts index 5bba75562..c7da85079 100644 --- a/packages/ocap-kernel/src/remotes/RemoteHandle.ts +++ b/packages/ocap-kernel/src/remotes/RemoteHandle.ts @@ -464,14 +464,16 @@ export class RemoteHandle implements EndpointHandle { // Set up timeout handling with AbortSignal const timeoutSignal = AbortSignal.timeout(30_000); + let abortHandler: (() => void) | undefined; const timeoutPromise = new Promise((_resolve, _reject) => { - timeoutSignal.addEventListener('abort', () => { + abortHandler = () => { // Clean up from pending redemptions map if (this.#pendingRedemptions.has(replyKey)) { this.#pendingRedemptions.delete(replyKey); } _reject(new Error('URL redemption timed out after 30 seconds')); - }); + }; + timeoutSignal.addEventListener('abort', abortHandler); }); try { @@ -487,6 +489,12 @@ export class RemoteHandle implements EndpointHandle { this.#pendingRedemptions.delete(replyKey); } throw error; + } finally { + // Clean up event listener to prevent unhandled rejection if operation + // completes before timeout + if (abortHandler) { + timeoutSignal.removeEventListener('abort', abortHandler); + } } } diff --git a/packages/ocap-kernel/src/remotes/network.ts b/packages/ocap-kernel/src/remotes/network.ts index 060be1a3d..cbe315af9 100644 --- a/packages/ocap-kernel/src/remotes/network.ts +++ b/packages/ocap-kernel/src/remotes/network.ts @@ -115,14 +115,26 @@ export async function initNetwork( timeoutMs = 10_000, ): Promise { const timeoutSignal = AbortSignal.timeout(timeoutMs); - return Promise.race([ - channel.msgStream.write(message), - new Promise((_resolve, reject) => { - timeoutSignal.addEventListener('abort', () => { - reject(new Error(`Message send timed out after ${timeoutMs}ms`)); - }); - }), - ]); + let abortHandler: (() => void) | undefined; + const timeoutPromise = new Promise((_resolve, reject) => { + abortHandler = () => { + reject(new Error(`Message send timed out after ${timeoutMs}ms`)); + }; + timeoutSignal.addEventListener('abort', abortHandler); + }); + + try { + return await Promise.race([ + channel.msgStream.write(message), + timeoutPromise, + ]); + } finally { + // Clean up event listener to prevent unhandled rejection if operation + // completes before timeout + if (abortHandler) { + timeoutSignal.removeEventListener('abort', abortHandler); + } + } } /** diff --git a/vitest.config.ts b/vitest.config.ts index c5d7b62b6..910b905c8 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -130,7 +130,7 @@ export default defineConfig({ 'packages/kernel-ui/**': { statements: 97.57, functions: 97.29, - branches: 93.25, + branches: 93.26, lines: 97.57, }, 'packages/kernel-utils/**': { From 198ac7ca72dec3cb0a0133e67001667e2e31aca5 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 17 Dec 2025 23:11:02 +0100 Subject: [PATCH 4/4] fix tests --- .../src/remotes/RemoteHandle.test.ts | 10 +- .../ocap-kernel/src/remotes/network.test.ts | 247 ++++++++++++++++++ vitest.config.ts | 6 +- 3 files changed, 258 insertions(+), 5 deletions(-) diff --git a/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts b/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts index fd3b7ae5d..4018fbab4 100644 --- a/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts +++ b/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts @@ -649,10 +649,13 @@ describe('RemoteHandle', () => { expect(AbortSignal.timeout).toHaveBeenCalledWith(30_000); expect(mockSignal?.timeoutMs).toBe(30_000); + // Wait for sendRemoteMessage to be called + await new Promise((resolve) => queueMicrotask(() => resolve())); + // Resolve the redemption to avoid hanging const sendCall = vi.mocked(mockRemoteComms.sendRemoteMessage).mock .calls[0]; - const sentMessage = JSON.parse(sendCall?.[1] as string); + const sentMessage = JSON.parse(sendCall![1]); const replyKey = sentMessage.params[1] as string; await remote.handleRemoteMessage( @@ -712,10 +715,13 @@ describe('RemoteHandle', () => { // Start a redemption const urlPromise = remote.redeemOcapURL(mockOcapURL); + // Wait for sendRemoteMessage to be called + await new Promise((resolve) => queueMicrotask(() => resolve())); + // Get the reply key that was used const sendCall = vi.mocked(mockRemoteComms.sendRemoteMessage).mock .calls[0]; - const sentMessage = JSON.parse(sendCall?.[1] as string); + const sentMessage = JSON.parse(sendCall![1]); const replyKey = sentMessage.params[1] as string; // Wait for the promise to be set up and event listener registered diff --git a/packages/ocap-kernel/src/remotes/network.test.ts b/packages/ocap-kernel/src/remotes/network.test.ts index c418b4eeb..5b99c16ab 100644 --- a/packages/ocap-kernel/src/remotes/network.test.ts +++ b/packages/ocap-kernel/src/remotes/network.test.ts @@ -1699,4 +1699,251 @@ describe('network.initNetwork', () => { }); }); }); + + describe('message send timeout', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('times out after 10 seconds when write hangs', async () => { + // Ensure isReconnecting returns false so we actually call writeWithTimeout + mockReconnectionManager.isReconnecting.mockReturnValue(false); + + const mockChannel = createMockChannel('peer-1'); + // Make write hang indefinitely - return a new hanging promise each time + mockChannel.msgStream.write.mockReset(); + mockChannel.msgStream.write.mockImplementation( + async () => + new Promise(() => { + // Never resolves - simulates hanging write + }), + ); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + const sendPromise = sendRemoteMessage('peer-1', 'test message'); + + // Wait for the promise to be set up and event listener registered + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Verify write was called (proves we're not returning early) + expect(mockChannel.msgStream.write).toHaveBeenCalled(); + + // Manually trigger the abort to simulate timeout + mockSignal?.abort(); + + // Wait for the abort handler to execute + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Note: sendRemoteMessage catches the timeout error and returns undefined + // The timeout error is handled internally and triggers connection loss handling + expect(await sendPromise).toBeUndefined(); + + // Verify that connection loss handling was triggered + expect(mockReconnectionManager.startReconnection).toHaveBeenCalled(); + }); + + it('does not timeout if write completes before timeout', async () => { + const mockChannel = createMockChannel('peer-1'); + mockChannel.msgStream.write.mockResolvedValue(undefined); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + const sendPromise = sendRemoteMessage('peer-1', 'test message'); + + // Write resolves immediately, so promise should resolve + expect(await sendPromise).toBeUndefined(); + + // Verify timeout signal was not aborted + expect(mockSignal?.aborted).toBe(false); + }); + + it('handles timeout errors and triggers connection loss handling', async () => { + // Ensure isReconnecting returns false so we actually call writeWithTimeout + mockReconnectionManager.isReconnecting.mockReturnValue(false); + + const mockChannel = createMockChannel('peer-1'); + // Make write hang indefinitely - return a new hanging promise each time + mockChannel.msgStream.write.mockReset(); + mockChannel.msgStream.write.mockImplementation( + async () => + new Promise(() => { + // Never resolves - simulates hanging write + }), + ); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + const sendPromise = sendRemoteMessage('peer-1', 'test message'); + + // Wait for the promise to be set up and event listener registered + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Manually trigger the abort to simulate timeout + mockSignal?.abort(); + + // Wait for the abort handler to execute + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Note: sendRemoteMessage catches the timeout error and returns undefined + // The timeout error is handled internally and triggers connection loss handling + expect(await sendPromise).toBeUndefined(); + + // Verify that connection loss handling was triggered + expect(mockReconnectionManager.startReconnection).toHaveBeenCalled(); + }); + + it('propagates write errors that occur before timeout', async () => { + // Ensure isReconnecting returns false so we actually call writeWithTimeout + mockReconnectionManager.isReconnecting.mockReturnValue(false); + + const mockChannel = createMockChannel('peer-1'); + const writeError = new Error('Write failed'); + mockChannel.msgStream.write.mockRejectedValue(writeError); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + const sendPromise = sendRemoteMessage('peer-1', 'test message'); + + // Write error occurs immediately + // Note: sendRemoteMessage catches write errors and returns undefined + // The error is handled internally and triggers connection loss handling + expect(await sendPromise).toBeUndefined(); + + // Verify that connection loss handling was triggered + expect(mockReconnectionManager.startReconnection).toHaveBeenCalled(); + }); + + it('writeWithTimeout uses AbortSignal.timeout with 10 second default', async () => { + const mockChannel = createMockChannel('peer-1'); + // Make write resolve immediately to avoid timeout + mockChannel.msgStream.write.mockResolvedValue(undefined); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + await sendRemoteMessage('peer-1', 'test message'); + + // Verify AbortSignal.timeout was called with 10 seconds (default) + expect(AbortSignal.timeout).toHaveBeenCalledWith(10_000); + expect(mockSignal?.timeoutMs).toBe(10_000); + }); + + it('error message includes correct timeout duration', async () => { + // Ensure isReconnecting returns false so we actually call writeWithTimeout + mockReconnectionManager.isReconnecting.mockReturnValue(false); + + const mockChannel = createMockChannel('peer-1'); + // Make write hang indefinitely - return a new hanging promise each time + mockChannel.msgStream.write.mockReset(); + mockChannel.msgStream.write.mockImplementation( + async () => + new Promise(() => { + // Never resolves - simulates hanging write + }), + ); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + + let mockSignal: ReturnType | undefined; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + mockSignal = makeAbortSignalMock(ms); + return mockSignal; + }); + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + const sendPromise = sendRemoteMessage('peer-1', 'test message'); + + // Wait for the promise to be set up and event listener registered + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Manually trigger the abort to simulate timeout + mockSignal?.abort(); + + // Wait for the abort handler to execute + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Note: sendRemoteMessage catches the timeout error and returns undefined + // The timeout error is handled internally + expect(await sendPromise).toBeUndefined(); + + // Verify that writeWithTimeout was called (the timeout error message includes the duration) + expect(mockChannel.msgStream.write).toHaveBeenCalled(); + }); + + it('handles multiple concurrent writes with timeout', async () => { + // Ensure isReconnecting returns false so we actually call writeWithTimeout + mockReconnectionManager.isReconnecting.mockReturnValue(false); + + const mockChannel = createMockChannel('peer-1'); + // Make write hang indefinitely - return a new hanging promise each time + mockChannel.msgStream.write.mockReset(); + mockChannel.msgStream.write.mockImplementation( + async () => + new Promise(() => { + // Never resolves - simulates hanging write + }), + ); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + + const mockSignals: ReturnType[] = []; + vi.spyOn(AbortSignal, 'timeout').mockImplementation((ms: number) => { + const signal = makeAbortSignalMock(ms); + mockSignals.push(signal); + return signal; + }); + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + const sendPromise1 = sendRemoteMessage('peer-1', 'message 1'); + const sendPromise2 = sendRemoteMessage('peer-1', 'message 2'); + + // Wait for the promises to be set up and event listeners registered + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Manually trigger the abort on all signals to simulate timeout + for (const signal of mockSignals) { + signal.abort(); + } + + // Wait for the abort handlers to execute + await new Promise((resolve) => queueMicrotask(() => resolve())); + + // Note: sendRemoteMessage catches the timeout error and returns undefined + // The timeout error is handled internally + expect(await sendPromise1).toBeUndefined(); + expect(await sendPromise2).toBeUndefined(); + + // Verify that writeWithTimeout was called for both messages + expect(mockChannel.msgStream.write).toHaveBeenCalledTimes(2); + }); + }); }); diff --git a/vitest.config.ts b/vitest.config.ts index 910b905c8..aacf38303 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -106,7 +106,7 @@ export default defineConfig({ 'packages/kernel-platforms/**': { statements: 99.38, functions: 100, - branches: 96.25, + branches: 96.2, lines: 99.38, }, 'packages/kernel-rpc-methods/**': { @@ -159,8 +159,8 @@ export default defineConfig({ }, 'packages/ocap-kernel/**': { statements: 96.53, - functions: 98.53, - branches: 97.73, + functions: 98.54, + branches: 97.59, lines: 96.53, }, 'packages/omnium-gatherum/**': {