From 7c49acf50e2161c45d2ef99c1f792b18de6d9a46 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Mon, 4 Nov 2024 21:11:31 +0100 Subject: [PATCH 1/2] Add source to ChromeRuntimeStream messages --- .../streams/src/ChromeRuntimeStream.test.ts | 55 +++++++++++++++---- packages/streams/src/ChromeRuntimeStream.ts | 32 ++++++++--- 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/packages/streams/src/ChromeRuntimeStream.test.ts b/packages/streams/src/ChromeRuntimeStream.test.ts index 406571e01..ea31fc64c 100644 --- a/packages/streams/src/ChromeRuntimeStream.test.ts +++ b/packages/streams/src/ChromeRuntimeStream.test.ts @@ -27,8 +27,10 @@ vi.mock('@endo/promise-kit', () => makePromiseKitMock()); const makeEnvelope = ( value: unknown, target: ChromeRuntimeStreamTarget, + source: ChromeRuntimeStreamTarget, ): MessageEnvelope => ({ target, + source, payload: value, }); @@ -41,10 +43,11 @@ const makeRuntime = (extensionId: string = EXTENSION_ID) => { const dispatchRuntimeMessage = ( message: unknown, target: ChromeRuntimeStreamTarget = ChromeRuntimeStreamTarget.Background, + source: ChromeRuntimeStreamTarget = ChromeRuntimeStreamTarget.Offscreen, senderId: string = extensionId, ): void => { listeners.forEach((listener) => - listener(makeEnvelope(message, target), { id: senderId }), + listener(makeEnvelope(message, target, source), { id: senderId }), ); }; @@ -74,6 +77,7 @@ describe('ChromeRuntimeReader', () => { const reader = new ChromeRuntimeReader( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); expect(reader).toBeInstanceOf(ChromeRuntimeReader); @@ -86,6 +90,7 @@ describe('ChromeRuntimeReader', () => { const reader = new ChromeRuntimeReader( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); const message = { foo: 'bar' }; @@ -99,12 +104,18 @@ describe('ChromeRuntimeReader', () => { const reader = new ChromeRuntimeReader( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); const nextP = reader.next(); const message1 = { foo: 'bar' }; const message2 = { fizz: 'buzz' }; - dispatchRuntimeMessage(message1, undefined, 'other-extension-id'); + dispatchRuntimeMessage( + message1, + ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, + 'other-extension-id', + ); dispatchRuntimeMessage(message2); expect(await nextP).toStrictEqual(makePendingResult(message2)); @@ -115,6 +126,7 @@ describe('ChromeRuntimeReader', () => { const reader = new ChromeRuntimeReader( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); const nextP = reader.next(); @@ -138,20 +150,18 @@ describe('ChromeRuntimeReader', () => { const reader = new ChromeRuntimeReader( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); const nextP = reader.next(); vi.spyOn(console, 'warn'); const message1 = { foo: 'bar' }; - // @ts-expect-error Intentional destructive testing - dispatchRuntimeMessage(message1, 'foo'); - - expect(console.warn).toHaveBeenCalledWith( - `ChromeRuntimeReader received message for unexpected target: ${stringify({ - target: 'foo', - payload: message1, - })}`, + dispatchRuntimeMessage( + message1, + // @ts-expect-error Intentional destructive testing + 'foo', + ChromeRuntimeStreamTarget.Offscreen, ); const message2 = { fizz: 'buzz' }; @@ -164,6 +174,7 @@ describe('ChromeRuntimeReader', () => { const reader = new ChromeRuntimeReader( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); expect(listeners).toHaveLength(1); @@ -180,6 +191,7 @@ describe('ChromeRuntimeReader', () => { const reader = new ChromeRuntimeReader( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, onEnd, ); @@ -197,6 +209,7 @@ describe('ChromeRuntimeWriter', () => { const writer = new ChromeRuntimeWriter( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); expect(writer).toBeInstanceOf(ChromeRuntimeWriter); @@ -208,6 +221,7 @@ describe('ChromeRuntimeWriter', () => { const writer = new ChromeRuntimeWriter( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); const message = { foo: 'bar' }; @@ -215,7 +229,11 @@ describe('ChromeRuntimeWriter', () => { expect(await nextP).toStrictEqual(makePendingResult(undefined)); expect(runtime.sendMessage).toHaveBeenCalledWith( - makeEnvelope(message, ChromeRuntimeStreamTarget.Background), + makeEnvelope( + message, + ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, + ), ); }); @@ -225,6 +243,7 @@ describe('ChromeRuntimeWriter', () => { const writer = new ChromeRuntimeWriter( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, onEnd, ); @@ -242,13 +261,25 @@ describe('ChromeRuntimeDuplexStream', () => { const duplexStreamP = ChromeRuntimeDuplexStream.make( asChromeRuntime(runtime), ChromeRuntimeStreamTarget.Background, - ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Offscreen, ); dispatchRuntimeMessage(makeAck()); return [await duplexStreamP, { runtime, dispatchRuntimeMessage }] as const; }; + it('throws an error when localTarget and remoteTarget are the same', async () => { + const { runtime } = makeRuntime(); + + await expect( + ChromeRuntimeDuplexStream.make( + asChromeRuntime(runtime), + ChromeRuntimeStreamTarget.Background, + ChromeRuntimeStreamTarget.Background, + ), + ).rejects.toThrow('localTarget and remoteTarget must be different'); + }); + it('constructs a ChromeRuntimeDuplexStream', async () => { const [duplexStream] = await makeDuplexStream(); diff --git a/packages/streams/src/ChromeRuntimeStream.ts b/packages/streams/src/ChromeRuntimeStream.ts index 903c77e6c..e77758d60 100644 --- a/packages/streams/src/ChromeRuntimeStream.ts +++ b/packages/streams/src/ChromeRuntimeStream.ts @@ -26,10 +26,12 @@ import type { Dispatchable } from './utils.js'; export enum ChromeRuntimeStreamTarget { Background = 'background', Offscreen = 'offscreen', + Devtools = 'devtools', } export type MessageEnvelope = { target: ChromeRuntimeStreamTarget; + source: ChromeRuntimeStreamTarget; payload: Payload; }; @@ -39,6 +41,7 @@ const isMessageEnvelope = ( typeof message === 'object' && message !== null && 'target' in message && + 'source' in message && 'payload' in message; /** @@ -56,11 +59,14 @@ export class ChromeRuntimeReader extends BaseReader { readonly #target: ChromeRuntimeStreamTarget; + readonly #source: ChromeRuntimeStreamTarget; + readonly #extensionId: string; constructor( runtime: ChromeRuntime, target: ChromeRuntimeStreamTarget, + source: ChromeRuntimeStreamTarget, onEnd?: OnEnd, ) { // eslint-disable-next-line prefer-const @@ -76,6 +82,7 @@ export class ChromeRuntimeReader extends BaseReader { this.#receiveInput = super.getReceiveInput(); this.#target = target; + this.#source = source; this.#extensionId = runtime.id; messageListener = this.#onMessage.bind(this); @@ -99,12 +106,7 @@ export class ChromeRuntimeReader extends BaseReader { return; } - if (message.target !== this.#target) { - console.warn( - `ChromeRuntimeReader received message for unexpected target: ${stringify( - message, - )}`, - ); + if (message.target !== this.#target || message.source !== this.#source) { return; } @@ -126,6 +128,7 @@ export class ChromeRuntimeWriter extends BaseWriter { constructor( runtime: ChromeRuntime, target: ChromeRuntimeStreamTarget, + source: ChromeRuntimeStreamTarget, onEnd?: OnEnd, ) { super( @@ -133,6 +136,7 @@ export class ChromeRuntimeWriter extends BaseWriter { async (value: Dispatchable) => { await runtime.sendMessage({ target, + source, payload: value, }); }, @@ -172,13 +176,19 @@ export class ChromeRuntimeDuplexStream< const reader = new ChromeRuntimeReader( runtime, localTarget, + remoteTarget, async () => { await writer.return(); }, ); - writer = new ChromeRuntimeWriter(runtime, remoteTarget, async () => { - await reader.return(); - }); + writer = new ChromeRuntimeWriter( + runtime, + remoteTarget, + localTarget, + async () => { + await reader.return(); + }, + ); super(reader, writer); harden(this); } @@ -188,6 +198,10 @@ export class ChromeRuntimeDuplexStream< localTarget: ChromeRuntimeStreamTarget, remoteTarget: ChromeRuntimeStreamTarget, ): Promise> { + if (localTarget === remoteTarget) { + throw new Error('localTarget and remoteTarget must be different'); + } + const stream = new ChromeRuntimeDuplexStream( runtime, localTarget, From 6481318cd3ec84c7932445b5212c9285bdca2814 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 5 Nov 2024 12:24:12 +0100 Subject: [PATCH 2/2] Debug log incorrect target or source --- packages/streams/src/ChromeRuntimeStream.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/streams/src/ChromeRuntimeStream.ts b/packages/streams/src/ChromeRuntimeStream.ts index e77758d60..9ad5e8f65 100644 --- a/packages/streams/src/ChromeRuntimeStream.ts +++ b/packages/streams/src/ChromeRuntimeStream.ts @@ -107,6 +107,11 @@ export class ChromeRuntimeReader extends BaseReader { } if (message.target !== this.#target || message.source !== this.#source) { + console.debug( + `ChromeRuntimeReader received message with incorrect target or source: ${stringify(message)}`, + `Expected target: ${this.#target}`, + `Expected source: ${this.#source}`, + ); return; }