Skip to content

fix: Add Source Filtering to ChromeRuntimeReader for Multi-Endpoint Support #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rekmarks merged 3 commits into from
Nov 6, 2024
Merged

fix: Add Source Filtering to ChromeRuntimeReader for Multi-Endpoint Support #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c49acf
Add source to ChromeRuntimeStream messages
sirtimid Nov 4, 2024
6481318
Debug log incorrect target or source
sirtimid Nov 5, 2024
5cdde38
Merge branch 'main' into sirtimid/chrome-duplex-stream
rekmarks Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions packages/streams/src/ChromeRuntimeStream.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import {
const makeEnvelope = (
value: unknown,
target: ChromeRuntimeStreamTarget,
source: ChromeRuntimeStreamTarget,
): MessageEnvelope<unknown> => ({
target,
source,
payload: value,
});

Expand All @@ -34,10 +36,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 }),
);
};

Expand Down Expand Up @@ -69,6 +72,7 @@ describe('ChromeRuntimeReader', () => {
const reader = new ChromeRuntimeReader(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
);

expect(reader).toBeInstanceOf(ChromeRuntimeReader);
Expand All @@ -81,6 +85,7 @@ describe('ChromeRuntimeReader', () => {
const reader = new ChromeRuntimeReader(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
);

const message = { foo: 'bar' };
Expand All @@ -94,12 +99,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));
Expand All @@ -110,6 +121,7 @@ describe('ChromeRuntimeReader', () => {
const reader = new ChromeRuntimeReader(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
);

const nextP = reader.next();
Expand All @@ -133,20 +145,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' };
Expand All @@ -159,6 +169,7 @@ describe('ChromeRuntimeReader', () => {
const reader = new ChromeRuntimeReader(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
);
expect(listeners).toHaveLength(1);

Expand All @@ -175,6 +186,7 @@ describe('ChromeRuntimeReader', () => {
const reader = new ChromeRuntimeReader(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
onEnd,
);

Expand All @@ -192,6 +204,7 @@ describe.concurrent('ChromeRuntimeWriter', () => {
const writer = new ChromeRuntimeWriter(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
);

expect(writer).toBeInstanceOf(ChromeRuntimeWriter);
Expand All @@ -203,14 +216,19 @@ describe.concurrent('ChromeRuntimeWriter', () => {
const writer = new ChromeRuntimeWriter(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
);

const message = { foo: 'bar' };
const nextP = writer.next(message);

expect(await nextP).toStrictEqual(makePendingResult(undefined));
expect(runtime.sendMessage).toHaveBeenCalledWith(
makeEnvelope(message, ChromeRuntimeStreamTarget.Background),
makeEnvelope(
message,
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
),
);
});

Expand All @@ -220,6 +238,7 @@ describe.concurrent('ChromeRuntimeWriter', () => {
const writer = new ChromeRuntimeWriter(
asChromeRuntime(runtime),
ChromeRuntimeStreamTarget.Background,
ChromeRuntimeStreamTarget.Offscreen,
onEnd,
);

Expand All @@ -237,13 +256,25 @@ describe.concurrent('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();

Expand Down
35 changes: 27 additions & 8 deletions packages/streams/src/ChromeRuntimeStream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ import type { Dispatchable } from './utils.js';
export enum ChromeRuntimeStreamTarget {
Background = 'background',
Offscreen = 'offscreen',
Devtools = 'devtools',
}

export type MessageEnvelope<Payload> = {
target: ChromeRuntimeStreamTarget;
source: ChromeRuntimeStreamTarget;
payload: Payload;
};

Expand All @@ -39,6 +41,7 @@ const isMessageEnvelope = (
typeof message === 'object' &&
message !== null &&
'target' in message &&
'source' in message &&
'payload' in message;

/**
Expand All @@ -56,11 +59,14 @@ export class ChromeRuntimeReader<Read extends Json> extends BaseReader<Read> {

readonly #target: ChromeRuntimeStreamTarget;

readonly #source: ChromeRuntimeStreamTarget;

readonly #extensionId: string;

constructor(
runtime: ChromeRuntime,
target: ChromeRuntimeStreamTarget,
source: ChromeRuntimeStreamTarget,
onEnd?: OnEnd,
) {
// eslint-disable-next-line prefer-const
Expand All @@ -76,6 +82,7 @@ export class ChromeRuntimeReader<Read extends Json> extends BaseReader<Read> {

this.#receiveInput = super.getReceiveInput();
this.#target = target;
this.#source = source;
this.#extensionId = runtime.id;

messageListener = this.#onMessage.bind(this);
Expand All @@ -99,11 +106,11 @@ export class ChromeRuntimeReader<Read extends Json> extends BaseReader<Read> {
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) {
console.debug(
`ChromeRuntimeReader received message with incorrect target or source: ${stringify(message)}`,
`Expected target: ${this.#target}`,
`Expected source: ${this.#source}`,
);
return;
}
Expand All @@ -126,13 +133,15 @@ export class ChromeRuntimeWriter<Write extends Json> extends BaseWriter<Write> {
constructor(
runtime: ChromeRuntime,
target: ChromeRuntimeStreamTarget,
source: ChromeRuntimeStreamTarget,
onEnd?: OnEnd,
) {
super(
'ChromeRuntimeWriter',
async (value: Dispatchable<Write>) => {
await runtime.sendMessage({
target,
source,
payload: value,
});
},
Expand Down Expand Up @@ -172,13 +181,19 @@ export class ChromeRuntimeDuplexStream<
const reader = new ChromeRuntimeReader<Read>(
runtime,
localTarget,
remoteTarget,
async () => {
await writer.return();
},
);
writer = new ChromeRuntimeWriter<Write>(runtime, remoteTarget, async () => {
await reader.return();
});
writer = new ChromeRuntimeWriter<Write>(
runtime,
remoteTarget,
localTarget,
async () => {
await reader.return();
},
);
super(reader, writer);
harden(this);
}
Expand All @@ -188,6 +203,10 @@ export class ChromeRuntimeDuplexStream<
localTarget: ChromeRuntimeStreamTarget,
remoteTarget: ChromeRuntimeStreamTarget,
): Promise<ChromeRuntimeDuplexStream<Read, Write>> {
if (localTarget === remoteTarget) {
throw new Error('localTarget and remoteTarget must be different');
}

const stream = new ChromeRuntimeDuplexStream<Read, Write>(
runtime,
localTarget,
Expand Down