From 49d232bd01340f8b163bedc098c0a12c3bc33e83 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 19:55:40 +0500 Subject: [PATCH 1/7] fix(astro): follow core's recorded launch options; stop listening on a document the page left MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hydration state is no longer a second copy of last-launch: a relaunch, and a repeat registration, build on core's lastLaunch for the host relaunchWith would pick, so a client launched through the umbrella directly is followed too. When a launch moves to another document the old document's listener is removed. Tests run against a faithful fake of core's carrier. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/astro/src/client.test.ts | 364 ++++++++++++++++----------- packages/astro/src/client.ts | 47 ++-- packages/astro/src/hydration.test.ts | 31 ++- packages/astro/src/hydration.ts | 13 + 4 files changed, 286 insertions(+), 169 deletions(-) diff --git a/packages/astro/src/client.test.ts b/packages/astro/src/client.test.ts index 481a008d..7840e4a4 100644 --- a/packages/astro/src/client.test.ts +++ b/packages/astro/src/client.test.ts @@ -1,21 +1,91 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { launch, relaunchUmbrella, listen, flush, order, liveClient } = vi.hoisted(() => ({ - liveClient: vi.fn(), - launch: vi.fn(), - relaunchUmbrella: vi.fn(), - listen: vi.fn(), - flush: vi.fn(), - order: [] as string[], -})); +// A faithful stand-in for core's carrier: `launch` records the kept options on success only (setCarrierLastLaunch), returns the live client +// for a repeat (launchCore's alreadyLaunched), and `relaunch` merges `{ ...lastLaunch.options, ...options }` on the host relaunchWith picks. +const world = vi.hoisted(() => { + type Slot = { + client?: unknown; + lastLaunch?: { appToken: string; options: Record }; + lastHost?: object; + }; + const slots = new Map(); + const slot = (host: object): Slot => { + let found = slots.get(host); + if (found === undefined) { + found = {}; + slots.set(host, found); + } + return found; + }; + const state = { + gate: undefined as Promise | undefined, + failNext: undefined as Error | undefined, + count: 0, + }; + const KEPT = ['document', 'window', 'onError', 'carrier']; + async function coreLaunch(appToken: string, options: Record = {}) { + const host = (options.carrier as object | undefined) ?? globalThis; + const existing = slot(host).client; + if (existing !== undefined) { + return existing; + } + await state.gate; + if (state.failNext !== undefined) { + const failure = state.failNext; + state.failNext = undefined; + throw failure; + } + const client = { id: ++state.count }; + const retained: Record = {}; + for (const key of KEPT) { + if (options[key] !== undefined) { + retained[key] = options[key]; + } + } + slot(host).client = client; + slot(host).lastLaunch = { appToken, options: retained }; + slot(globalThis).lastHost = host === globalThis ? undefined : host; + return client; + } + async function coreRelaunch(options?: Record) { + const host = + (options?.carrier as object | undefined) ?? slot(globalThis).lastHost ?? globalThis; + const last = slot(host).lastLaunch; + if (last === undefined) { + throw new Error('relaunch before launch'); + } + slot(host).client = undefined; + return await coreLaunch(last.appToken, { ...last.options, ...options }); + } + return { + slots, + slot, + state, + launch: vi.fn(coreLaunch), + relaunchUmbrella: vi.fn(coreRelaunch), + coreLaunch, + listen: vi.fn(), + flush: vi.fn(), + stop: vi.fn(), + order: [] as string[], + }; +}); +const { launch, relaunchUmbrella, listen, flush, stop, order } = world; vi.mock('@bugsee/bugsee', () => ({ launch, relaunch: relaunchUmbrella })); -vi.mock('@bugsee/core', () => ({ getCarrierClient: liveClient })); +vi.mock('@bugsee/core', () => ({ + getCarrier: (host: object = globalThis) => world.slot(host), + getCarrierClient: (host: object = globalThis) => world.slot(host).client, +})); vi.mock('./hydration', () => ({ - listenForAstroHydrationErrors: listen, - flushQueuedAstroHydrationErrors: flush, + listenForAstroHydrationErrors: world.listen, + flushQueuedAstroHydrationErrors: world.flush, + stopListeningForAstroHydrationErrors: world.stop, })); -// The client module remembers the last registration's hydration options (like core's relaunch keeps them), so each test gets a fresh one. +const doc = (id: string) => ({ id }) as unknown as Document; +const lastListen = () => listen.mock.lastCall?.[0] as Record; + +// The client module keeps the document it last listened on, so each test gets a fresh module. let registerClient: typeof import('./client').registerClient; let relaunch: typeof import('./client').relaunch; @@ -26,18 +96,21 @@ describe('registerClient', () => { }); afterEach(() => { - launch.mockReset(); - relaunchUmbrella.mockReset(); + world.slots.clear(); + world.state.gate = undefined; + world.state.failNext = undefined; + world.state.count = 0; + launch.mockClear(); + relaunchUmbrella.mockClear(); listen.mockReset(); flush.mockReset(); - liveClient.mockReset(); + stop.mockReset(); order.length = 0; }); it('launches the browser SDK with the appToken + options, returns the client', async () => { - const client = { id: 'browser' }; - launch.mockReturnValue(client); - await expect(registerClient('tok', { captureLogs: false })).resolves.toBe(client); + const client = await registerClient('tok', { captureLogs: false }); + expect(client).toBe(world.slot(globalThis).client); expect(launch).toHaveBeenCalledWith('tok', { captureLogs: false }); }); @@ -48,9 +121,9 @@ describe('registerClient', () => { it('listens BEFORE the launch (an island error during launch is queued) and flushes the queue once the client exists', async () => { listen.mockImplementation(() => order.push('listen')); - launch.mockImplementation(async () => { + launch.mockImplementationOnce(async (token, options) => { order.push('launch'); - return {}; + return await world.coreLaunch(token, options as Record); }); flush.mockImplementation(() => order.push('flush')); await registerClient('tok'); @@ -58,63 +131,42 @@ describe('registerClient', () => { }); it('relaunch() is wrapped in the same window: listen first, flush in a finally (the carrier client is cleared during the restart)', async () => { + await registerClient('tok'); listen.mockImplementation(() => order.push('listen')); - relaunchUmbrella.mockImplementation(async () => { - order.push('relaunch'); - return {}; - }); flush.mockImplementation(() => order.push('flush')); - await relaunch({ replay: false }); - expect(order).toEqual(['listen', 'relaunch', 'flush']); + await relaunch({ replay: false } as never); + expect(order).toEqual(['listen', 'flush']); expect(relaunchUmbrella).toHaveBeenCalledWith({ replay: false }); - relaunchUmbrella.mockRejectedValue(new Error('relaunch failed')); + world.state.failNext = new Error('relaunch failed'); order.length = 0; await expect(relaunch()).rejects.toThrow('relaunch failed'); expect(order).toEqual(['listen', 'flush']); }); it("a fresh registration takes its document from its OWN options: an explicit one, else the bound window's", async () => { - const doc = { id: 'explicit' } as unknown as Document; - const windowDoc = { id: 'window' } as unknown as Document; - launch.mockResolvedValueOnce({ id: 1 }); - await registerClient('tok', { document: doc } as never); - expect(listen).toHaveBeenLastCalledWith({ target: doc }); - // a client that was stopped (or never came up) gives a NEW client: core launches afresh with only these options, so the old document, - // carrier and sink are not carried into it + await registerClient('tok', { document: doc('explicit') } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit') }); + world.slot(globalThis).client = undefined; // stopped: the next registration is a fresh launch const carrier = {}; - launch.mockResolvedValueOnce({ id: 2 }); - await registerClient('tok', { window: { document: windowDoc }, carrier } as never); - expect(listen).toHaveBeenLastCalledWith({ target: windowDoc, carrier }); + await registerClient('tok', { window: { document: doc('window') }, carrier } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('window'), carrier }); }); - it('a failed launch or relaunch does not become the remembered state (core records last-launch options only on success)', async () => { - const doc = { id: 'explicit' } as unknown as Document; - launch.mockResolvedValueOnce({ id: 1 }); - relaunchUmbrella.mockRejectedValueOnce(new Error('boom')); - relaunchUmbrella.mockResolvedValue({ id: 2 }); - await registerClient('tok', { document: doc } as never); - await expect(relaunch({ document: { id: 'failed' } } as never)).rejects.toThrow('boom'); - await relaunch(); - expect(listen).toHaveBeenLastCalledWith({ target: doc }); - launch.mockRejectedValueOnce(new Error('nope')); - await expect(registerClient('tok', { document: { id: 'failed2' } } as never)).rejects.toThrow( - 'nope', - ); + it('a failed launch or relaunch leaves what the next relaunch builds on untouched (core records last-launch options only on success)', async () => { + await registerClient('tok', { document: doc('explicit') } as never); + world.state.failNext = new Error('boom'); + await expect(relaunch({ document: doc('failed') } as never)).rejects.toThrow('boom'); await relaunch(); - expect(listen).toHaveBeenLastCalledWith({ target: doc }); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit') }); }); it('an explicit undefined on a relaunch overwrites the kept value, as core spreads it', async () => { - const windowDoc = { id: 'window' } as unknown as Document; - launch.mockResolvedValue({ id: 1 }); - relaunchUmbrella.mockResolvedValue({ id: 2 }); - await registerClient('tok', { document: { id: 'explicit' } } as never); - await relaunch({ document: undefined, window: { document: windowDoc } } as never); - expect(listen).toHaveBeenLastCalledWith({ target: windowDoc }); + await registerClient('tok', { document: doc('explicit') } as never); + await relaunch({ document: undefined, window: { document: doc('window') } } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('window') }); }); it('a sandboxed window whose document getter throws leaves the listener on its default target', async () => { - launch.mockResolvedValue({}); const hostile = { get document(): never { throw new Error('sandboxed'); @@ -125,179 +177,185 @@ describe('registerClient', () => { }); it('a plain relaunch() keeps the earlier document, carrier and onError (core keeps them), and an explicit one wins', async () => { - const doc = { id: 'explicit' } as unknown as Document; const carrier = {}; const onError = vi.fn(); - launch.mockResolvedValue({}); - relaunchUmbrella.mockResolvedValue({}); - await registerClient('tok', { document: doc, carrier, onError } as never); + await registerClient('tok', { document: doc('explicit'), carrier, onError } as never); await relaunch(); - expect(listen).toHaveBeenLastCalledWith({ target: doc, carrier, onError }); - expect(flush).toHaveBeenLastCalledWith({ target: doc, carrier, onError }); - const other = { id: 'other' } as unknown as Document; - await relaunch({ document: other } as never); - expect(listen).toHaveBeenLastCalledWith({ target: other, carrier, onError }); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit'), carrier, onError }); + expect(flush).toHaveBeenLastCalledWith({ target: doc('explicit'), carrier, onError }); + await relaunch({ document: doc('other') } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('other'), carrier, onError }); }); it("a plain relaunch() after a bound WINDOW listens and flushes on that window's document (the same EventTarget as registerClient)", async () => { - const windowDoc = {} as Document; - launch.mockResolvedValue({}); - relaunchUmbrella.mockResolvedValue({}); - await registerClient('tok', { window: { document: windowDoc } } as never); - await relaunch({ replay: false }); - expect(listen).toHaveBeenLastCalledWith({ target: windowDoc }); - expect(flush).toHaveBeenLastCalledWith({ target: windowDoc }); + await registerClient('tok', { window: { document: doc('window') } } as never); + await relaunch({ replay: false } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('window') }); + expect(flush).toHaveBeenLastCalledWith({ target: doc('window') }); }); it("a relaunch({ window }) after an explicit document keeps that document, because launch prefers `document` over the window's", async () => { - const doc = { id: 'explicit' } as unknown as Document; - const windowDoc = { id: 'window' } as unknown as Document; - launch.mockResolvedValue({}); - relaunchUmbrella.mockResolvedValue({}); - await registerClient('tok', { document: doc } as never); - await relaunch({ window: { document: windowDoc } } as never); - expect(listen).toHaveBeenLastCalledWith({ target: doc }); - expect(flush).toHaveBeenLastCalledWith({ target: doc }); - // and an explicit document on the relaunch still wins over the kept one - const other = { id: 'other' } as unknown as Document; - await relaunch({ document: other } as never); - expect(listen).toHaveBeenLastCalledWith({ target: other }); + await registerClient('tok', { document: doc('explicit') } as never); + await relaunch({ window: { document: doc('window') } } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit') }); + expect(flush).toHaveBeenLastCalledWith({ target: doc('explicit') }); + await relaunch({ document: doc('other') } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('other') }); }); it('hands a custom carrier to the hydration listener, so it looks where the client lives', async () => { const carrier = {}; - launch.mockResolvedValue({}); await registerClient('tok', { carrier } as never); expect(listen).toHaveBeenCalledWith({ carrier }); expect(flush).toHaveBeenCalledWith({ carrier }); }); it('ends the queueing window even when the launch fails, and still rejects with the launch error', async () => { - const failure = new Error('launch failed'); - launch.mockRejectedValue(failure); - await expect(registerClient('tok')).rejects.toBe(failure); + world.state.failNext = new Error('launch failed'); + await expect(registerClient('tok')).rejects.toThrow('launch failed'); expect(flush).toHaveBeenCalledTimes(1); }); it('always states the carrier and sink, so an explicit undefined replaces what the listener kept (core relaunches on the global carrier)', async () => { - launch.mockResolvedValue({ id: 1 }); - relaunchUmbrella.mockResolvedValue({ id: 2 }); await registerClient('tok', { carrier: {}, onError: vi.fn() } as never); await relaunch({ carrier: undefined, onError: undefined } as never); - const passed = listen.mock.lastCall?.[0] as Record; + const passed = lastListen(); expect(Object.hasOwn(passed, 'carrier') && passed.carrier === undefined).toBe(true); expect(Object.hasOwn(passed, 'onError') && passed.onError === undefined).toBe(true); }); it('a relaunch started while a registration is still launching waits for it and builds on its options (core awaits the pending launch)', async () => { - const doc = { id: 'explicit' } as unknown as Document; - let finish: (client: unknown) => void = () => undefined; - launch.mockReturnValue(new Promise((resolve) => (finish = resolve))); - relaunchUmbrella.mockResolvedValue({ id: 2 }); - const registering = registerClient('tok', { document: doc } as never); + let open: () => void = () => undefined; + world.state.gate = new Promise((resolve) => (open = resolve)); + const registering = registerClient('tok', { document: doc('explicit') } as never); const relaunching = relaunch(); - finish({ id: 1 }); + open(); await Promise.all([registering, relaunching]); - expect(listen).toHaveBeenLastCalledWith({ target: doc }); - await relaunch(); - expect(listen).toHaveBeenLastCalledWith({ target: doc }); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit') }); }); it('a relaunch still goes ahead when the launch it waited for failed', async () => { - launch.mockRejectedValue(new Error('nope')); - relaunchUmbrella.mockResolvedValue({ id: 2 }); - const registering = registerClient('tok'); - const relaunching = relaunch(); - await expect(registering).rejects.toThrow('nope'); - await expect(relaunching).resolves.toEqual({ id: 2 }); + await registerClient('tok', { document: doc('explicit') } as never); + world.state.failNext = new Error('nope'); + const failing = relaunch(); // the restart fails ... + const following = relaunch(); // ... and the next one still runs, on what the last SUCCESSFUL launch recorded + await expect(failing).rejects.toThrow('nope'); + await expect(following).resolves.toBeDefined(); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit') }); }); it("only OWN option properties count, as in core's spread: an inherited document is not the launch's document", async () => { - const own = { id: 'own' } as unknown as Document; - const inherited = { id: 'inherited' } as unknown as Document; - launch.mockResolvedValue({ id: 1 }); - relaunchUmbrella.mockResolvedValue({ id: 2 }); - await registerClient('tok', { document: own } as never); - await relaunch(Object.create({ document: inherited }) as never); - expect(listen).toHaveBeenLastCalledWith({ target: own }); + await registerClient('tok', { document: doc('own') } as never); + await relaunch(Object.create({ document: doc('inherited') }) as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('own') }); }); - it('a repeat registration (a client is live) merges only its DEFINED options onto the listener and remembers nothing, wherever the client was launched', async () => { - const doc = { id: 'explicit' } as unknown as Document; + it('a repeat registration (a client is live) merges only its DEFINED options onto the listener, wherever the client was launched', async () => { const carrier = {}; const sink = vi.fn(); - const live = { id: 'live' }; - launch.mockResolvedValueOnce(live); - relaunchUmbrella.mockResolvedValue({ id: 2 }); - await registerClient('tok', { document: doc, carrier } as never); - liveClient.mockImplementation((host: unknown) => (host === carrier ? live : undefined)); - launch.mockResolvedValueOnce(live); - // the realistic late call: only a sink (functions cannot ride the injected script), a stated undefined is core-discarded + await registerClient('tok', { document: doc('explicit'), carrier } as never); + // the realistic late call: only a sink (functions cannot ride the injected script); a stated undefined was discarded by core await registerClient('tok', { carrier, onError: sink, document: undefined } as never); - const passed = listen.mock.lastCall?.[0] as Record; - expect(passed).toEqual({ target: doc, carrier, onError: sink }); + const passed = lastListen(); + expect(passed).toEqual({ target: doc('explicit'), carrier, onError: sink }); expect(Object.hasOwn(passed, 'document')).toBe(false); - expect(flush).toHaveBeenLastCalledWith(passed); // the listen is settled at once: the client exists + expect(flush).toHaveBeenLastCalledWith(passed); // settled at once: the client exists expect(launch).toHaveBeenLastCalledWith('tok', { carrier, onError: sink, document: undefined }); - // nothing was remembered from the repeat: a plain relaunch is back on the first launch's options + // core recorded nothing for the repeat, so a relaunch is back on the first launch's options await relaunch(); - expect(listen).toHaveBeenLastCalledWith({ target: doc, carrier }); - expect( - Object.hasOwn(listen.mock.lastCall?.[0] as object, 'onError') && - (listen.mock.lastCall?.[0] as { onError?: unknown }).onError, - ).toBe(undefined); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit'), carrier }); + expect(Object.hasOwn(lastListen(), 'onError') && lastListen().onError).toBe(undefined); }); - it('a repeat registration with no options keeps the listener on the remembered carrier and sink (it never states undefined)', async () => { + it('a repeat registration with no options keeps the listener on the recorded carrier and sink (it never states undefined)', async () => { const carrier = {}; const sink = vi.fn(); - const live = { id: 'live' }; - launch.mockResolvedValue(live); await registerClient('tok', { carrier, onError: sink } as never); - liveClient.mockReturnValue(live); await registerClient('tok', { carrier } as never); expect(listen).toHaveBeenCalledTimes(2); - const passed = listen.mock.lastCall?.[0] as Record; - expect(passed).toEqual({ carrier, onError: sink }); - expect(Object.keys(passed).sort()).toEqual(['carrier', 'onError']); + expect(lastListen()).toEqual({ carrier, onError: sink }); + expect(Object.keys(lastListen()).sort()).toEqual(['carrier', 'onError']); }); it('registrations overlapping a launch in flight wait for it, then see the live client (the carrier is set only at the end of a launch)', async () => { const carrier = {}; const sink = vi.fn(); - const live = { id: 'live' }; - let finish: (client: unknown) => void = () => undefined; - launch.mockReturnValueOnce(new Promise((resolve) => (finish = resolve))); - launch.mockResolvedValueOnce(live); + let open: () => void = () => undefined; + world.state.gate = new Promise((resolve) => (open = resolve)); const first = registerClient('tok', { carrier, onError: sink } as never); const second = registerClient('tok', { carrier } as never); await new Promise((resolve) => setTimeout(resolve, 0)); expect(launch).toHaveBeenCalledTimes(1); // the second has not started: it waits - liveClient.mockReturnValue(live); // the first launch ends: the carrier now holds the client - finish(live); + open(); await Promise.all([first, second]); expect(launch).toHaveBeenCalledTimes(2); - const passed = listen.mock.lastCall?.[0] as Record; - expect(passed).toEqual({ carrier, onError: sink }); // not wiped by the second call + expect(lastListen()).toEqual({ carrier, onError: sink }); // not wiped by the second call }); it('asks the carrier the launch would use: the option, else the global', async () => { const carrier = {}; - launch.mockResolvedValue({ id: 1 }); await registerClient('tok', { carrier } as never); - expect(liveClient).toHaveBeenLastCalledWith(carrier); + // a client lives on `carrier` only: a registration on the global carrier is a FRESH launch (its own slot) and states undefined await registerClient('tok'); - expect(liveClient).toHaveBeenLastCalledWith(globalThis); + expect(world.slot(globalThis).client).toBeDefined(); + expect(Object.hasOwn(lastListen(), 'carrier')).toBe(true); + expect(lastListen().carrier).toBe(undefined); }); it('passes the onError sink to the hydration listener, and a relaunch that gives none keeps it (core keeps onError)', async () => { const onError = vi.fn(); - launch.mockResolvedValue({ id: 1 }); - relaunchUmbrella.mockResolvedValue({ id: 2 }); await registerClient('tok', { onError }); expect(listen).toHaveBeenCalledTimes(1); expect(listen).toHaveBeenCalledWith({ onError }); await relaunch(); expect(listen).toHaveBeenLastCalledWith({ onError }); }); + + it('a relaunch builds on what CORE recorded, so a client launched through the umbrella directly is followed too', async () => { + await world.coreLaunch('tok', { document: doc('umbrella') }); // not through registerClient + await relaunch({ window: { document: doc('window') } } as never); + expect(listen).toHaveBeenLastCalledWith({ target: doc('umbrella') }); + expect(flush).toHaveBeenLastCalledWith({ target: doc('umbrella') }); + }); + + it('a relaunch follows the host core would pick: the carrier option, else the one the last launch remembered', async () => { + const carrier = {}; + await registerClient('tok', { document: doc('on-carrier'), carrier } as never); + await relaunch(); // core: slot(globalThis).lastHost === carrier + expect(listen).toHaveBeenLastCalledWith({ target: doc('on-carrier'), carrier }); + const other = {}; + await world.coreLaunch('tok', { document: doc('on-other'), carrier: other }); + await relaunch({ carrier: other } as never); // an explicit carrier wins + expect(lastListen().target).toEqual(doc('on-other')); + }); + + it('when the client moves to another document, the old document stops listening', async () => { + await registerClient('tok', { document: doc('first') } as never); + expect(stop).not.toHaveBeenCalled(); + await relaunch({ document: doc('second') } as never); + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('first')); + await relaunch(); // same document again: nothing to remove + expect(stop).toHaveBeenCalledTimes(1); + }); + + it('a failed move leaves the old listener alone (nothing moved)', async () => { + await registerClient('tok', { document: doc('first') } as never); + world.state.failNext = new Error('boom'); + await expect(relaunch({ document: doc('failed') } as never)).rejects.toThrow('boom'); + expect(stop).not.toHaveBeenCalled(); + }); + + it('a repeat registration (core ignores its document) leaves the old listener alone', async () => { + await registerClient('tok', { document: doc('first') } as never); + await registerClient('tok', { document: doc('repeat') } as never); + expect(stop).not.toHaveBeenCalled(); + }); + + it('a launch with no document at all leaves the old listener alone (it stays on its default target)', async () => { + await registerClient('tok', { document: doc('first') } as never); + world.slot(globalThis).client = undefined; // stopped + await registerClient('tok'); + expect(stop).not.toHaveBeenCalled(); + }); }); diff --git a/packages/astro/src/client.ts b/packages/astro/src/client.ts index 386b4ee9..77921fdc 100644 --- a/packages/astro/src/client.ts +++ b/packages/astro/src/client.ts @@ -10,8 +10,12 @@ import { launch, relaunch as umbrellaRelaunch, } from '@bugsee/bugsee'; -import { getCarrierClient } from '@bugsee/core'; -import { flushQueuedAstroHydrationErrors, listenForAstroHydrationErrors } from './hydration'; +import { getCarrier, getCarrierClient } from '@bugsee/core'; +import { + flushQueuedAstroHydrationErrors, + listenForAstroHydrationErrors, + stopListeningForAstroHydrationErrors, +} from './hydration'; /** Options for the Astro client (browser) composition — the batteries-included browser umbrella options. */ export interface AstroClientOptions extends BugseeLaunchOptionsWithPerformance {} @@ -45,13 +49,19 @@ type Remembered = { carrier?: object; }; -// What the last SUCCESSFUL launch handed in, like core's `lastLaunch`. `document` and `window` stay RAW: which one the launch uses is -// decided by precedence, so a relaunch that adds only a window must not displace a kept explicit document. -let remembered: Remembered = {}; // The launch still in flight. Every call waits for it first, as core's `relaunchWith` does: only then is it known whether a client is live -// (the carrier is set at the END of a launch) and what the last launch remembered. +// (the carrier is set at the END of a launch) and what the last launch recorded. let inFlight: Promise = Promise.resolve(); -// Known limit (accepted): the hydration listener of a document the page has moved away from stays attached (as before this tracking). +// The document the last launch listened on, so a launch that moves to another one can stop listening on the old one. +let activeTarget: EventTarget | undefined; + +// What core recorded for the last SUCCESSFUL launch on `host` (`setCarrierLastLaunch`: the kept options, `undefined` ones dropped). This +// is the very state `relaunchWith` merges from, so Astro cannot disagree with core, whichever entry point made that launch. `document` and +// `window` stay RAW: which one the launch uses is decided by precedence, so a relaunch that adds only a window must not displace a kept +// explicit document. +function recordedBy(host: object): Remembered { + return pick(getCarrier(host).lastLaunch?.options as HydrationInput, true); +} function pick(options: HydrationInput, definedOnly: boolean): Remembered { const picked: Remembered = {}; @@ -73,13 +83,13 @@ type Plan = { base: Remembered; repeat: boolean }; * Open the hydration listener's launch window, run `start`, and close the window in a `finally`: an error dispatched while the client is * (re)starting is queued, and the flush runs whether the start succeeded or failed. Waits for the launch in flight first, then asks `plan`: * - * - a relaunch builds on the remembered options (core's `relaunchWith` keeps `document`, `window`, `carrier` and `onError`; an explicit key + * - a relaunch builds on what core recorded (core's `relaunchWith` keeps `document`, `window`, `carrier` and `onError`; an explicit key * wins, even an explicit `undefined`); * - a fresh launch builds on nothing (core's plain launch carries nothing over); - * - a REPEAT (a client is live, so core ignores the call and returns it) builds on the remembered options and takes only the DEFINED keys - * it was given: its stated `undefined` was discarded by core, and the listener must not lose what it has. Nothing is remembered from it. + * - a REPEAT (a client is live, so core ignores the call and returns it) builds on what core recorded and takes only the DEFINED keys + * it was given: its stated `undefined` was discarded by core, and the listener must not lose what it has. Core records nothing for it. * - * The remembered state moves only once a non-repeat start succeeded. + * A non-repeat start that moved to another document stops listening on the old one. */ function withHydrationWindow( options: HydrationInput, @@ -118,8 +128,11 @@ async function openWindow( listenForAstroHydrationErrors(hydration); try { const client = await start(); - if (!repeat) { - remembered = next; + if (!repeat && target !== undefined) { + if (activeTarget !== undefined && activeTarget !== target) { + stopListeningForAstroHydrationErrors(activeTarget); // the page moved to another document + } + activeTarget = target; } return client; } finally { @@ -140,7 +153,7 @@ export async function registerClient( // a client already live on the carrier the launch would look at: core ignores this call () => getCarrierClient(options.carrier ?? globalThis) !== undefined - ? { base: remembered, repeat: true } + ? { base: recordedBy(options.carrier ?? globalThis), repeat: true } : { base: {}, repeat: false }, () => launch(appToken, options), ); @@ -153,7 +166,11 @@ export async function registerClient( export async function relaunch(options?: Parameters[0]): Promise { return await withHydrationWindow( options, - () => ({ base: remembered, repeat: false }), + () => ({ + // the host core's `relaunchWith` would pick: the carrier option, else the one the last launch left, else the global + base: recordedBy(options?.carrier ?? getCarrier().lastHost ?? globalThis), + repeat: false, + }), () => umbrellaRelaunch(options), ); } diff --git a/packages/astro/src/hydration.test.ts b/packages/astro/src/hydration.test.ts index 87a2e931..0f5b2b81 100644 --- a/packages/astro/src/hydration.test.ts +++ b/packages/astro/src/hydration.test.ts @@ -1,7 +1,11 @@ import type { Bugsee } from '@bugsee/bugsee'; import { setCarrierClient } from '@bugsee/core'; import { describe, expect, it, vi } from 'vitest'; -import { flushQueuedAstroHydrationErrors, listenForAstroHydrationErrors } from './hydration'; +import { + flushQueuedAstroHydrationErrors, + listenForAstroHydrationErrors, + stopListeningForAstroHydrationErrors, +} from './hydration'; const clientWith = (logException = vi.fn(() => Promise.resolve(undefined))) => ({ logException }) as unknown as Bugsee; @@ -301,3 +305,28 @@ describe('listenForAstroHydrationErrors', () => { expect(getClient).toHaveBeenCalledTimes(1); }); }); + +describe('stopListeningForAstroHydrationErrors', () => { + it('removes the listener of a target the page moved away from, and forgets its queue', () => { + const target = new EventTarget(); + const logException = vi.fn(() => Promise.resolve(undefined)); + const options = { target, getClient: () => clientWith(logException) }; + listenForAstroHydrationErrors(options); + stopListeningForAstroHydrationErrors(target); + target.dispatchEvent(hydrationError(new Error('late'))); + expect(logException).not.toHaveBeenCalled(); + // a later registration on the same target is a NEW listener (the old state is gone), reporting exactly once + listenForAstroHydrationErrors(options); + target.dispatchEvent(hydrationError(new Error('again'))); + expect(logException).toHaveBeenCalledTimes(1); + }); + + it('is a no-op for a target nobody is listening on, and does not touch other targets', () => { + const other = new EventTarget(); + const logException = vi.fn(() => Promise.resolve(undefined)); + listenForAstroHydrationErrors({ target: other, getClient: () => clientWith(logException) }); + expect(() => stopListeningForAstroHydrationErrors(new EventTarget())).not.toThrow(); + other.dispatchEvent(hydrationError(new Error('x'))); + expect(logException).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/astro/src/hydration.ts b/packages/astro/src/hydration.ts index 099cab5d..2780aac8 100644 --- a/packages/astro/src/hydration.ts +++ b/packages/astro/src/hydration.ts @@ -114,6 +114,19 @@ export function listenForAstroHydrationErrors(options: AstroHydrationOptions = { }; } +/** + * Stop listening on `target` (the page moved to another document): removes the listener and forgets its queue and options. A no-op for a + * target that is not being listened on. + */ +export function stopListeningForAstroHydrationErrors(target: EventTarget): void { + const state = listening.get(target); + if (state === undefined) { + return; + } + target.removeEventListener(EVENT, state.listener); + listening.delete(target); +} + /** * Settle one registration's launch window (call once its launch has settled, success or failure): report what was queued if a client * exists now, and close the window once the last pending registration has settled (afterwards an error with no client is dropped, not From 5c56464691aedaec02420545af2c67fca3e5ccd5 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 20:00:37 +0500 Subject: [PATCH 2/7] fix(astro): track the listened document per carrier host, default document included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the first cut: one module-level slot let a launch on one carrier remove the listener another carrier's client depends on; the default page document was never tracked; a failed launch leaked the listener it had opened; a repeat attached a listener on a document core had ignored; and a relaunch read core's record before core's own pending launch had finished. Documents are released only when no host is on them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/astro/src/client.test.ts | 168 ++++++++++++++++++++++++--- packages/astro/src/client.ts | 79 ++++++++----- packages/astro/src/hydration.test.ts | 13 ++- 3 files changed, 210 insertions(+), 50 deletions(-) diff --git a/packages/astro/src/client.test.ts b/packages/astro/src/client.test.ts index 7840e4a4..40b70f77 100644 --- a/packages/astro/src/client.test.ts +++ b/packages/astro/src/client.test.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // for a repeat (launchCore's alreadyLaunched), and `relaunch` merges `{ ...lastLaunch.options, ...options }` on the host relaunchWith picks. const world = vi.hoisted(() => { type Slot = { + pendingLaunch?: Promise; client?: unknown; lastLaunch?: { appToken: string; options: Record }; lastHost?: object; @@ -23,29 +24,40 @@ const world = vi.hoisted(() => { count: 0, }; const KEPT = ['document', 'window', 'onError', 'carrier']; - async function coreLaunch(appToken: string, options: Record = {}) { + function coreLaunch(appToken: string, options: Record = {}): Promise { const host = (options.carrier as object | undefined) ?? globalThis; const existing = slot(host).client; if (existing !== undefined) { - return existing; + return Promise.resolve(existing); } - await state.gate; - if (state.failNext !== undefined) { - const failure = state.failNext; - state.failNext = undefined; - throw failure; - } - const client = { id: ++state.count }; - const retained: Record = {}; - for (const key of KEPT) { - if (options[key] !== undefined) { - retained[key] = options[key]; + // like core, a launch in flight is visible on the carrier (getCarrierPendingLaunch) until it settles + const run = (async () => { + await state.gate; + if (state.failNext !== undefined) { + const failure = state.failNext; + state.failNext = undefined; + throw failure; } - } - slot(host).client = client; - slot(host).lastLaunch = { appToken, options: retained }; - slot(globalThis).lastHost = host === globalThis ? undefined : host; - return client; + const client = { id: ++state.count }; + const retained: Record = {}; + for (const key of KEPT) { + if (options[key] !== undefined) { + retained[key] = options[key]; + } + } + slot(host).client = client; + slot(host).lastLaunch = { appToken, options: retained }; + slot(globalThis).lastHost = host === globalThis ? undefined : host; + return client; + })(); + slot(host).pendingLaunch = run; + const clear = () => { + if (slot(host).pendingLaunch === run) { + slot(host).pendingLaunch = undefined; + } + }; + run.then(clear, clear); + return run; } async function coreRelaunch(options?: Record) { const host = @@ -75,6 +87,7 @@ vi.mock('@bugsee/bugsee', () => ({ launch, relaunch: relaunchUmbrella })); vi.mock('@bugsee/core', () => ({ getCarrier: (host: object = globalThis) => world.slot(host), getCarrierClient: (host: object = globalThis) => world.slot(host).client, + getCarrierPendingLaunch: (host: object = globalThis) => world.slot(host).pendingLaunch, })); vi.mock('./hydration', () => ({ listenForAstroHydrationErrors: world.listen, @@ -96,6 +109,7 @@ describe('registerClient', () => { }); afterEach(() => { + delete (globalThis as { document?: unknown }).document; world.slots.clear(); world.state.gate = undefined; world.state.failNext = undefined; @@ -343,13 +357,21 @@ describe('registerClient', () => { await registerClient('tok', { document: doc('first') } as never); world.state.failNext = new Error('boom'); await expect(relaunch({ document: doc('failed') } as never)).rejects.toThrow('boom'); - expect(stop).not.toHaveBeenCalled(); + expect(stop).not.toHaveBeenCalledWith(doc('first')); + // ... but the document the failed launch had started listening on is released, not leaked + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('failed')); }); it('a repeat registration (core ignores its document) leaves the old listener alone', async () => { await registerClient('tok', { document: doc('first') } as never); await registerClient('tok', { document: doc('repeat') } as never); expect(stop).not.toHaveBeenCalled(); + // core ignored that document, so nothing listens on it: the listener stays on the document the client uses + expect(lastListen().target).toEqual(doc('first')); + expect( + listen.mock.calls.some(([passed]) => (passed as { target?: unknown }).target === undefined), + ).toBe(false); }); it('a launch with no document at all leaves the old listener alone (it stays on its default target)', async () => { @@ -358,4 +380,112 @@ describe('registerClient', () => { await registerClient('tok'); expect(stop).not.toHaveBeenCalled(); }); + + it('the default page document counts as the launch document: default then explicit stops listening on it', async () => { + const page = doc('page'); + (globalThis as { document?: unknown }).document = page; + await registerClient('tok'); // listens on globalThis.document by default + await relaunch({ document: doc('second') } as never); + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(page); + }); + + it('explicit then default: the client is back on the page document, so the explicit one stops', async () => { + const page = doc('page'); + (globalThis as { document?: unknown }).document = page; + await registerClient('tok', { document: doc('first') } as never); + world.slot(globalThis).client = undefined; // stopped + await registerClient('tok'); // no document: the default one + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('first')); + }); + + it('a failed first launch releases the listener it opened', async () => { + world.state.failNext = new Error('boom'); + await expect(registerClient('tok', { document: doc('first') } as never)).rejects.toThrow( + 'boom', + ); + expect(stop).toHaveBeenCalledWith(doc('first')); + }); + + it("a relaunch waits for a launch in flight that Astro did not start (core's relaunchWith awaits the carrier's pending launch)", async () => { + let open: () => void = () => undefined; + world.state.gate = new Promise((resolve) => (open = resolve)); + const external = world.coreLaunch('tok', { document: doc('umbrella') }); // through the umbrella, not registerClient + const relaunching = relaunch({ window: { document: doc('window') } } as never); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(relaunchUmbrella).not.toHaveBeenCalled(); // not started: it waits for the pending launch + open(); + await Promise.all([external, relaunching]); + expect(listen).toHaveBeenLastCalledWith({ target: doc('umbrella') }); + }); + + it('a pending launch that fails does not stop the relaunch from going ahead', async () => { + await registerClient('tok', { document: doc('explicit') } as never); + world.slot(globalThis).client = undefined; + let open: () => void = () => undefined; + world.state.gate = new Promise((resolve) => (open = resolve)); + world.state.failNext = new Error('external failed'); + const external = world.coreLaunch('tok', {}); // pending on the carrier, and about to fail + external.catch(() => undefined); + const relaunching = relaunch(); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(relaunchUmbrella).not.toHaveBeenCalled(); // it is waiting for the pending launch + world.state.gate = undefined; + open(); + await expect(relaunching).resolves.toBeDefined(); + expect(listen).toHaveBeenLastCalledWith({ target: doc('explicit') }); + }); + + it('two carriers on two documents: a launch on one never stops the other (core keeps its state per carrier)', async () => { + const a = {}; + const b = {}; + await registerClient('tok', { carrier: a, document: doc('docA') } as never); + await registerClient('tok', { carrier: b, document: doc('docB') } as never); + expect(stop).not.toHaveBeenCalled(); + await relaunch({ carrier: b, document: doc('docB2') } as never); // B moves; A is untouched + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('docB')); + }); + + it('two carriers sharing one document: it stops only when the last of them has left it', async () => { + const a = {}; + const b = {}; + const shared = doc('shared'); + await registerClient('tok', { carrier: a, document: shared } as never); + await registerClient('tok', { carrier: b, document: shared } as never); + await relaunch({ carrier: b, document: doc('elsewhere') } as never); // A is still on it + expect(stop).not.toHaveBeenCalled(); + await relaunch({ carrier: a, document: doc('also-elsewhere') } as never); // now nobody is + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(shared); + }); + + it('a failed launch on a carrier does not release a document another carrier still uses', async () => { + const a = {}; + const b = {}; + const shared = doc('shared'); + await registerClient('tok', { carrier: a, document: shared } as never); + world.state.failNext = new Error('boom'); + await expect(registerClient('tok', { carrier: b, document: shared } as never)).rejects.toThrow( + 'boom', + ); + expect(stop).not.toHaveBeenCalled(); + }); + + it('a repeat that states onError undefined (core discarded it) keeps the recorded sink', async () => { + const sink = vi.fn(); + await registerClient('tok', { onError: sink }); + await registerClient('tok', { onError: undefined }); + expect(lastListen()).toEqual({ onError: sink }); + expect(lastListen().onError).toBe(sink); + }); + + it('a failed relaunch onto the document the host is already on does not stop it', async () => { + const same = doc('same'); + await registerClient('tok', { document: same } as never); + world.state.failNext = new Error('boom'); + await expect(relaunch({ document: same } as never)).rejects.toThrow('boom'); + expect(stop).not.toHaveBeenCalled(); + }); }); diff --git a/packages/astro/src/client.ts b/packages/astro/src/client.ts index 77921fdc..cd9fd9be 100644 --- a/packages/astro/src/client.ts +++ b/packages/astro/src/client.ts @@ -10,7 +10,7 @@ import { launch, relaunch as umbrellaRelaunch, } from '@bugsee/bugsee'; -import { getCarrier, getCarrierClient } from '@bugsee/core'; +import { getCarrier, getCarrierClient, getCarrierPendingLaunch } from '@bugsee/core'; import { flushQueuedAstroHydrationErrors, listenForAstroHydrationErrors, @@ -52,24 +52,39 @@ type Remembered = { // The launch still in flight. Every call waits for it first, as core's `relaunchWith` does: only then is it known whether a client is live // (the carrier is set at the END of a launch) and what the last launch recorded. let inFlight: Promise = Promise.resolve(); -// The document the last launch listened on, so a launch that moves to another one can stop listening on the old one. -let activeTarget: EventTarget | undefined; +// The document each carrier host's last launch listened on (core's state is per host), so a launch that moves to another document can stop +// listening on the old one, unless another host still uses it. +const activeTargets = new Map(); + +function release(target: EventTarget): void { + for (const used of activeTargets.values()) { + if (used === target) { + return; // another carrier's client is still on this document + } + } + stopListeningForAstroHydrationErrors(target); +} // What core recorded for the last SUCCESSFUL launch on `host` (`setCarrierLastLaunch`: the kept options, `undefined` ones dropped). This // is the very state `relaunchWith` merges from, so Astro cannot disagree with core, whichever entry point made that launch. `document` and // `window` stay RAW: which one the launch uses is decided by precedence, so a relaunch that adds only a window must not displace a kept // explicit document. function recordedBy(host: object): Remembered { - return pick(getCarrier(host).lastLaunch?.options as HydrationInput, true); + return pick(getCarrier(host).lastLaunch?.options as HydrationInput, false); } -function pick(options: HydrationInput, definedOnly: boolean): Remembered { +// Which options a call contributes. A repeat's `document` and `window` were ignored by core (the client is on the recorded ones), so only its +// `onError` and `carrier` count, and only when defined: a stated `undefined` was discarded too. +const LAUNCH_KEYS = ['document', 'window', 'onError', 'carrier'] as const; +const REPEAT_KEYS = ['onError', 'carrier'] as const; + +function pick(options: HydrationInput, repeat: boolean): Remembered { const picked: Remembered = {}; - for (const key of ['document', 'window', 'onError', 'carrier'] as const) { + for (const key of repeat ? REPEAT_KEYS : LAUNCH_KEYS) { if ( options !== undefined && Object.hasOwn(options, key) && - !(definedOnly && options[key] === undefined) + !(repeat && options[key] === undefined) ) { (picked as Record)[key] = options[key]; } @@ -81,25 +96,29 @@ type Plan = { base: Remembered; repeat: boolean }; /** * Open the hydration listener's launch window, run `start`, and close the window in a `finally`: an error dispatched while the client is - * (re)starting is queued, and the flush runs whether the start succeeded or failed. Waits for the launch in flight first, then asks `plan`: + * (re)starting is queued, and the flush runs whether the start succeeded or failed. Waits for the launch in flight first (Astro's own, and + * the one core still has pending on the carrier, as `relaunchWith` does), then asks `plan`: * * - a relaunch builds on what core recorded (core's `relaunchWith` keeps `document`, `window`, `carrier` and `onError`; an explicit key * wins, even an explicit `undefined`); * - a fresh launch builds on nothing (core's plain launch carries nothing over); - * - a REPEAT (a client is live, so core ignores the call and returns it) builds on what core recorded and takes only the DEFINED keys - * it was given: its stated `undefined` was discarded by core, and the listener must not lose what it has. Core records nothing for it. + * - a REPEAT (a client is live, so core ignores the call and returns it) builds on what core recorded and takes only its DEFINED + * `onError` and `carrier`: core ignored its document and window, and discarded a stated `undefined`. Core records nothing for it. * - * A non-repeat start that moved to another document stops listening on the old one. + * A start that moved the host to another document stops listening on the old one; one that failed releases the new one. */ function withHydrationWindow( options: HydrationInput, - plan: () => Plan, + hostOf: () => object, + plan: (host: object) => Plan, start: () => Promise, ): Promise { const previous = inFlight; const run = (async () => { await previous.then(undefined, () => undefined); - return await openWindow(options, plan(), start); + const host = hostOf(); + await getCarrierPendingLaunch(host)?.then(undefined, () => undefined); + return await openWindow(options, host, plan(host), start); })(); inFlight = run; return run; @@ -107,11 +126,14 @@ function withHydrationWindow( async function openWindow( options: HydrationInput, + host: object, { base, repeat }: Plan, start: () => Promise, ): Promise { const next = { ...base, ...pick(options, repeat) }; const target = launchDocument(next.document, next.window); + // Where the listener really attaches: an unset target means the page document (`hydration.ts`). + const tracked = target ?? (globalThis as { document?: EventTarget }).document; // A launch states `onError` and `carrier` ALWAYS: the listener merges what it is given over what it kept, so an absent key would bring // back a carrier or sink that this launch no longer has. A repeat states only what it has. const hydration = { @@ -128,13 +150,19 @@ async function openWindow( listenForAstroHydrationErrors(hydration); try { const client = await start(); - if (!repeat && target !== undefined) { - if (activeTarget !== undefined && activeTarget !== target) { - stopListeningForAstroHydrationErrors(activeTarget); // the page moved to another document + if (!repeat && tracked !== undefined) { + const left = activeTargets.get(host); + activeTargets.set(host, tracked); + if (left !== undefined && left !== tracked) { + release(left); // the page moved to another document } - activeTarget = target; } return client; + } catch (error) { + if (!repeat && tracked !== undefined) { + release(tracked); // opened for a launch that never happened (a no-op when some host is on it) + } + throw error; } finally { flushQueuedAstroHydrationErrors(hydration); // success: report what queued; failure: end the window, nothing to report to } @@ -150,10 +178,11 @@ export async function registerClient( ): Promise { return await withHydrationWindow( options, - // a client already live on the carrier the launch would look at: core ignores this call - () => - getCarrierClient(options.carrier ?? globalThis) !== undefined - ? { base: recordedBy(options.carrier ?? globalThis), repeat: true } + () => options.carrier ?? globalThis, // the carrier the launch would look at + (host) => + // a client already live there: core ignores this call + getCarrierClient(host) !== undefined + ? { base: recordedBy(host), repeat: true } : { base: {}, repeat: false }, () => launch(appToken, options), ); @@ -166,11 +195,9 @@ export async function registerClient( export async function relaunch(options?: Parameters[0]): Promise { return await withHydrationWindow( options, - () => ({ - // the host core's `relaunchWith` would pick: the carrier option, else the one the last launch left, else the global - base: recordedBy(options?.carrier ?? getCarrier().lastHost ?? globalThis), - repeat: false, - }), + // the host core's `relaunchWith` picks: the carrier option, else the one the last launch left, else the global + () => options?.carrier ?? getCarrier().lastHost ?? globalThis, + (host) => ({ base: recordedBy(host), repeat: false }), () => umbrellaRelaunch(options), ); } diff --git a/packages/astro/src/hydration.test.ts b/packages/astro/src/hydration.test.ts index 0f5b2b81..d4b73b5d 100644 --- a/packages/astro/src/hydration.test.ts +++ b/packages/astro/src/hydration.test.ts @@ -310,15 +310,18 @@ describe('stopListeningForAstroHydrationErrors', () => { it('removes the listener of a target the page moved away from, and forgets its queue', () => { const target = new EventTarget(); const logException = vi.fn(() => Promise.resolve(undefined)); - const options = { target, getClient: () => clientWith(logException) }; - listenForAstroHydrationErrors(options); + // no client yet: the error is QUEUED inside the launch window + listenForAstroHydrationErrors({ target, getClient: () => undefined }); + target.dispatchEvent(hydrationError(new Error('queued'))); stopListeningForAstroHydrationErrors(target); target.dispatchEvent(hydrationError(new Error('late'))); - expect(logException).not.toHaveBeenCalled(); - // a later registration on the same target is a NEW listener (the old state is gone), reporting exactly once + // a later registration on the same target is a NEW listener with an empty queue: flushing it reports nothing of the old one + const options = { target, getClient: () => clientWith(logException) }; listenForAstroHydrationErrors(options); + flushQueuedAstroHydrationErrors(options); + expect(logException).not.toHaveBeenCalled(); target.dispatchEvent(hydrationError(new Error('again'))); - expect(logException).toHaveBeenCalledTimes(1); + expect(logException).toHaveBeenCalledTimes(1); // exactly one listener }); it('is a no-op for a target nobody is listening on, and does not touch other targets', () => { From 7b86dd501d1f7ff8cc7ed6325d4f5d9f1e879d94 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 20:02:40 +0500 Subject: [PATCH 3/7] fix(astro): a repeat registers the document it listens on when the host has none tracked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/astro/src/client.test.ts | 16 ++++++++++++++++ packages/astro/src/client.ts | 3 +++ 2 files changed, 19 insertions(+) diff --git a/packages/astro/src/client.test.ts b/packages/astro/src/client.test.ts index 40b70f77..c87f0b74 100644 --- a/packages/astro/src/client.test.ts +++ b/packages/astro/src/client.test.ts @@ -488,4 +488,20 @@ describe('registerClient', () => { await expect(relaunch({ document: same } as never)).rejects.toThrow('boom'); expect(stop).not.toHaveBeenCalled(); }); + + it('a repeat registration on a client Astro never launched still tracks the document it listens on, so a later move releases it', async () => { + await world.coreLaunch('tok', { document: doc('umbrella') }); // through the umbrella, not registerClient + await registerClient('tok'); // a repeat: listens on the recorded document + expect(lastListen().target).toEqual(doc('umbrella')); + await relaunch({ document: doc('moved') } as never); + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('umbrella')); + }); + + it('a repeat never moves or releases what is already tracked for its host', async () => { + await registerClient('tok', { document: doc('first') } as never); + await registerClient('tok', { onError: vi.fn() }); + await relaunch(); // still the first document: nothing stops + expect(stop).not.toHaveBeenCalled(); + }); }); diff --git a/packages/astro/src/client.ts b/packages/astro/src/client.ts index cd9fd9be..398c381b 100644 --- a/packages/astro/src/client.ts +++ b/packages/astro/src/client.ts @@ -150,6 +150,9 @@ async function openWindow( listenForAstroHydrationErrors(hydration); try { const client = await start(); + if (repeat && tracked !== undefined && !activeTargets.has(host)) { + activeTargets.set(host, tracked); // a client Astro did not launch: remember where its listener is, so a later move can release it + } if (!repeat && tracked !== undefined) { const left = activeTargets.get(host); activeTargets.set(host, tracked); From e09993825ea00b413a8ab9e3f7ad91a7fe87d081 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 20:03:14 +0500 Subject: [PATCH 4/7] fix(astro): one tracking path for launches and repeats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repeat's document comes from core's record, so it is tracked like a launch's and releases the one Astro knew when the client was moved behind its back. Drops two conditions release() already makes redundant. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/astro/src/client.test.ts | 12 ++++++++++++ packages/astro/src/client.ts | 11 +++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/astro/src/client.test.ts b/packages/astro/src/client.test.ts index c87f0b74..c86a4fbf 100644 --- a/packages/astro/src/client.test.ts +++ b/packages/astro/src/client.test.ts @@ -504,4 +504,16 @@ describe('registerClient', () => { await relaunch(); // still the first document: nothing stops expect(stop).not.toHaveBeenCalled(); }); + + it('a repeat after the umbrella moved the client to another document releases the one Astro knew', async () => { + await registerClient('tok', { document: doc('first') } as never); + world.slot(globalThis).lastLaunch = { + appToken: 'tok', + options: { document: doc('moved-by-umbrella') }, + }; + await registerClient('tok'); // a repeat: the live client is now recorded on another document + expect(lastListen().target).toEqual(doc('moved-by-umbrella')); + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('first')); + }); }); diff --git a/packages/astro/src/client.ts b/packages/astro/src/client.ts index 398c381b..c27e530f 100644 --- a/packages/astro/src/client.ts +++ b/packages/astro/src/client.ts @@ -150,14 +150,13 @@ async function openWindow( listenForAstroHydrationErrors(hydration); try { const client = await start(); - if (repeat && tracked !== undefined && !activeTargets.has(host)) { - activeTargets.set(host, tracked); // a client Astro did not launch: remember where its listener is, so a later move can release it - } - if (!repeat && tracked !== undefined) { + if (tracked !== undefined) { + // The document this host's client is now on: for a repeat, the one core recorded (a client Astro did not launch, or one the umbrella + // moved), for a launch, the one it just started on. const left = activeTargets.get(host); activeTargets.set(host, tracked); - if (left !== undefined && left !== tracked) { - release(left); // the page moved to another document + if (left !== undefined) { + release(left); // the page may have moved to another document (a no-op while any host is still on `left`) } } return client; From 8416f1b945073fedbcd7c4688749cd00af5d8fa8 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 20:08:16 +0500 Subject: [PATCH 5/7] test(astro): hydration listeners against the real registry and carrier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unit tests mock listen/stop, so they cannot see which documents still have a listener. These run the real hydration module on real EventTargets with the real core carrier (umbrella launch/relaunch stubbed): a failed move releases its destination, the default page document is released, two carriers do not silence each other, and a repeat on an umbrella-launched client is tracked. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/astro/src/client.integration.test.ts | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 packages/astro/src/client.integration.test.ts diff --git a/packages/astro/src/client.integration.test.ts b/packages/astro/src/client.integration.test.ts new file mode 100644 index 00000000..cb6d2fbf --- /dev/null +++ b/packages/astro/src/client.integration.test.ts @@ -0,0 +1,138 @@ +import { getCarrier, getCarrierClient, setCarrierClient, setCarrierLastLaunch } from '@bugsee/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// The REAL hydration listeners (on real EventTargets) and the REAL core carrier; only the umbrella's launch/relaunch are stubbed, and they +// record on the real carrier the way core does. This is what the unit tests (mocked listen/stop) cannot see: which documents really +// still have a listener, and where an error really goes. +type Opts = Record & { carrier?: object }; +const state = { failNext: undefined as Error | undefined }; +const clients: Array<{ logException: ReturnType }> = []; + +vi.mock('@bugsee/bugsee', () => { + const launch = async (appToken: string, options: Opts = {}) => { + const host = options.carrier ?? globalThis; + const existing = getCarrierClient(host); + if (existing !== undefined) { + return existing; + } + if (state.failNext !== undefined) { + const failure = state.failNext; + state.failNext = undefined; + throw failure; + } + const client = { logException: vi.fn(() => Promise.resolve(undefined)) }; + clients.push(client); + setCarrierClient(client, host); + setCarrierLastLaunch(appToken, options, host); + getCarrier().lastHost = host === globalThis ? undefined : host; + return client; + }; + const relaunch = async (options?: Opts) => { + const host = options?.carrier ?? getCarrier().lastHost ?? globalThis; + const last = getCarrier(host).lastLaunch; + if (last === undefined) { + throw new Error('relaunch before launch'); + } + getCarrier(host).client = undefined; // relaunchWith stops the running client first + return await launch(last.appToken, { ...(last.options as Opts), ...options }); + }; + return { launch, relaunch }; +}); + +const hydrationError = (error: unknown): Event => + new CustomEvent('astro:hydration-error', { detail: { error }, bubbles: true }); +const page = (): EventTarget => new EventTarget(); + +let registerClient: typeof import('./client').registerClient; +let relaunch: typeof import('./client').relaunch; + +describe('hydration listeners against the real registry', () => { + beforeEach(async () => { + vi.resetModules(); + ({ registerClient, relaunch } = await import('./client')); + }); + afterEach(() => { + const carrier = getCarrier(); + carrier.client = undefined; + carrier.lastLaunch = undefined; + carrier.lastHost = undefined; + delete (globalThis as { document?: unknown }).document; + clients.length = 0; + state.failNext = undefined; + }); + + const reported = (client: { logException: ReturnType }, error: unknown) => + client.logException.mock.calls.some(([thrown]) => thrown === error); + + it('a failed move releases the destination: its errors are not reported into the recovered session', async () => { + const a = page(); + const b = page(); + const first = await registerClient('tok', { document: a } as never); + state.failNext = new Error('launch failed'); + await expect(relaunch({ document: b } as never)).rejects.toThrow('launch failed'); + const recovered = (await relaunch()) as unknown as (typeof clients)[number]; // back on `a`, from core's record + const lateB = new Error('b'); + b.dispatchEvent(hydrationError(lateB)); + expect(reported(recovered, lateB)).toBe(false); + const onA = new Error('a'); + a.dispatchEvent(hydrationError(onA)); + expect(reported(recovered, onA)).toBe(true); + expect(reported(first as never, onA)).toBe(false); // the first client is gone: one report, to the live one + }); + + it('the default page document is released when the client moves to an explicit one', async () => { + const pageDocument = page(); + (globalThis as { document?: unknown }).document = pageDocument; + await registerClient('tok'); + const moved = page(); + const client = (await relaunch({ + document: moved, + } as never)) as unknown as (typeof clients)[number]; + const onPage = new Error('page'); + pageDocument.dispatchEvent(hydrationError(onPage)); + expect(reported(client, onPage)).toBe(false); + const onMoved = new Error('moved'); + moved.dispatchEvent(hydrationError(onMoved)); + expect(reported(client, onMoved)).toBe(true); + }); + + it('two carriers on two documents each keep reporting; one moving does not silence the other', async () => { + const carrierA = {}; + const carrierB = {}; + const docA = page(); + const docB = page(); + const clientA = (await registerClient('tok', { + carrier: carrierA, + document: docA, + } as never)) as never as (typeof clients)[number]; + await registerClient('tok', { + carrier: carrierB, + document: docB, + } as never); + const movedB = page(); + await relaunch({ carrier: carrierB, document: movedB } as never); + const onA = new Error('a'); + docA.dispatchEvent(hydrationError(onA)); + expect(reported(clientA, onA)).toBe(true); + const onOldB = new Error('old b'); + docB.dispatchEvent(hydrationError(onOldB)); + expect(clients.some((client) => reported(client, onOldB))).toBe(false); // B left docB: no client gets its errors + const onNewB = new Error('new b'); + movedB.dispatchEvent(hydrationError(onNewB)); + expect(clients.some((client) => reported(client, onNewB))).toBe(true); + }); + + it('a repeat for a client the umbrella launched is tracked, so moving it releases the document', async () => { + const umbrellaDoc = page(); + const { launch } = await import('@bugsee/bugsee'); + await launch('tok', { document: umbrellaDoc } as never); + await registerClient('tok'); // a repeat: attaches on the recorded document + const moved = page(); + const client = (await relaunch({ + document: moved, + } as never)) as unknown as (typeof clients)[number]; + const stale = new Error('stale'); + umbrellaDoc.dispatchEvent(hydrationError(stale)); + expect(reported(client, stale)).toBe(false); + }); +}); From 3156a7c5dd9201f3d03ea01e7812bf0df902255f Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 20:14:52 +0500 Subject: [PATCH 6/7] fix(astro): key the tracked document by the host the client now lives on; leave a shared listener alone until the launch succeeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit relaunch({ carrier: undefined }) moves the client to the global carrier, so the entry is keyed by next.carrier ?? globalThis and the lookup host's old document is released too. A launch onto a document another host's live client listens on no longer merges its carrier and sink onto that listener before it has succeeded, so a failed sibling launch cannot misroute the live client's errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/astro/src/client.integration.test.ts | 36 +++++++++++ packages/astro/src/client.test.ts | 63 +++++++++++++++++++ packages/astro/src/client.ts | 47 +++++++++++--- 3 files changed, 137 insertions(+), 9 deletions(-) diff --git a/packages/astro/src/client.integration.test.ts b/packages/astro/src/client.integration.test.ts index cb6d2fbf..7eb26c31 100644 --- a/packages/astro/src/client.integration.test.ts +++ b/packages/astro/src/client.integration.test.ts @@ -135,4 +135,40 @@ describe('hydration listeners against the real registry', () => { umbrellaDoc.dispatchEvent(hydrationError(stale)); expect(reported(client, stale)).toBe(false); }); + + it("a launch that fails on a document another carrier's client listens on leaves that client reporting", async () => { + const carrierA = {}; + const carrierB = {}; + const shared = page(); + const sinkA = vi.fn(); + const clientA = (await registerClient('tok', { + carrier: carrierA, + document: shared, + onError: sinkA, + } as never)) as never as (typeof clients)[number]; + state.failNext = new Error('B failed'); + await expect( + registerClient('tok', { carrier: carrierB, document: shared } as never), + ).rejects.toThrow('B failed'); + const error = new Error('island'); + shared.dispatchEvent(hydrationError(error)); + expect(reported(clientA, error)).toBe(true); // not dropped: the listener still looks on A's carrier + }); + + it('after relaunch({ carrier: undefined }) a later move releases the document the client left', async () => { + const custom = {}; + const docA = page(); + await registerClient('tok', { carrier: custom, document: docA } as never); + await relaunch({ carrier: undefined } as never); // now on globalThis + const docB = page(); + const client = (await relaunch({ + document: docB, + } as never)) as unknown as (typeof clients)[number]; + const stale = new Error('stale'); + docA.dispatchEvent(hydrationError(stale)); + expect(reported(client, stale)).toBe(false); + const fresh = new Error('fresh'); + docB.dispatchEvent(hydrationError(fresh)); + expect(reported(client, fresh)).toBe(true); + }); }); diff --git a/packages/astro/src/client.test.ts b/packages/astro/src/client.test.ts index c86a4fbf..668e42a0 100644 --- a/packages/astro/src/client.test.ts +++ b/packages/astro/src/client.test.ts @@ -516,4 +516,67 @@ describe('registerClient', () => { expect(stop).toHaveBeenCalledTimes(1); expect(stop).toHaveBeenLastCalledWith(doc('first')); }); + + it('after relaunch({ carrier: undefined }) the client lives on the global carrier, so a later move releases the document it left', async () => { + const custom = {}; + await registerClient('tok', { carrier: custom, document: doc('docA') } as never); + await relaunch({ carrier: undefined } as never); // core: the new client is on globalThis; lastHost is cleared + expect(world.slot(globalThis).client).toBeDefined(); + await relaunch({ document: doc('docB') } as never); // looks up globalThis now + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('docA')); + }); + + it("a launch that fails on a document another host's live client listens on does not touch that listener", async () => { + const a = {}; + const b = {}; + const shared = doc('shared'); + await registerClient('tok', { carrier: a, document: shared, onError: vi.fn() } as never); + listen.mockClear(); + flush.mockClear(); + world.state.failNext = new Error('boom'); + await expect(registerClient('tok', { carrier: b, document: shared } as never)).rejects.toThrow( + 'boom', + ); + expect(listen).not.toHaveBeenCalled(); // B's carrier and sink never reached the live listener + expect(flush).not.toHaveBeenCalled(); + expect(stop).not.toHaveBeenCalled(); + }); + + it('the same launch, once it succeeds, listens on the shared document and settles that registration', async () => { + const a = {}; + const b = {}; + const shared = doc('shared'); + await registerClient('tok', { carrier: a, document: shared } as never); + listen.mockClear(); + flush.mockClear(); + await registerClient('tok', { carrier: b, document: shared } as never); + expect(listen).toHaveBeenCalledTimes(1); + expect(flush).toHaveBeenCalledTimes(1); + expect(lastListen()).toEqual({ target: shared, carrier: b }); + }); + + it('a relaunch that changes the carrier AND the document releases the document the lookup host was on', async () => { + const custom = {}; + await registerClient('tok', { carrier: custom, document: doc('docA') } as never); + await relaunch({ carrier: undefined, document: doc('docB') } as never); + expect(stop).toHaveBeenCalledTimes(1); + expect(stop).toHaveBeenLastCalledWith(doc('docA')); + }); + + it('a relaunch that stays on its own document still listens BEFORE the restart (errors during it are queued)', async () => { + const same = doc('same'); + await registerClient('tok', { document: same } as never); + order.length = 0; + listen.mockImplementation(() => order.push('listen')); + relaunchUmbrella.mockImplementationOnce(async (options) => { + order.push('restart'); + return await world.coreLaunch('tok', { + ...(world.slot(globalThis).lastLaunch?.options ?? {}), + ...(options as object), + }); + }); + await relaunch(); + expect(order).toEqual(['listen', 'restart']); + }); }); diff --git a/packages/astro/src/client.ts b/packages/astro/src/client.ts index c27e530f..4f846e3d 100644 --- a/packages/astro/src/client.ts +++ b/packages/astro/src/client.ts @@ -56,6 +56,16 @@ let inFlight: Promise = Promise.resolve(); // listening on the old one, unless another host still uses it. const activeTargets = new Map(); +// Another host's client is already on this document, so its listener serves that client until this launch has actually succeeded. +function usedByOther(target: EventTarget, host: object): boolean { + for (const [other, used] of activeTargets) { + if (other !== host && used === target) { + return true; + } + } + return false; +} + function release(target: EventTarget): void { for (const used of activeTargets.values()) { if (used === target) { @@ -146,17 +156,34 @@ async function openWindow( ...(target !== undefined ? { target } : {}), }; // Astro swallows an island's hydration failure into an `astro:hydration-error` event; nothing else would see it. Listen BEFORE the - // launch resolves (an error dispatched meanwhile is queued) and flush once the client exists. - listenForAstroHydrationErrors(hydration); + // launch resolves (an error dispatched meanwhile is queued) and flush once the client exists. Not so on a document another host's client + // already listens on: merging this launch's carrier and sink onto that listener now would misroute the live client's errors if this launch + // then failed, so it waits for the launch to succeed (the live client's own listener covers the meantime). + let listened = tracked === undefined || !usedByOther(tracked, host); + if (listened) { + listenForAstroHydrationErrors(hydration); + } try { const client = await start(); + if (!listened) { + listenForAstroHydrationErrors(hydration); + listened = true; + } if (tracked !== undefined) { - // The document this host's client is now on: for a repeat, the one core recorded (a client Astro did not launch, or one the umbrella - // moved), for a launch, the one it just started on. - const left = activeTargets.get(host); - activeTargets.set(host, tracked); - if (left !== undefined) { - release(left); // the page may have moved to another document (a no-op while any host is still on `left`) + // The document this client is now on, under the host it now lives on: a relaunch with `carrier: undefined` moves it to the global + // carrier (core's spread, then `setCarrierLastLaunch` clears `lastHost`), whichever host was looked up to find the OLD client. + // For a repeat, the one core recorded (a client Astro did not launch, or one the umbrella moved); for a launch, the one it started on. + const live = next.carrier ?? globalThis; + const lefts = [activeTargets.get(live)]; + if (live !== host) { + lefts.push(activeTargets.get(host)); + activeTargets.delete(host); + } + activeTargets.set(live, tracked); + for (const left of lefts) { + if (left !== undefined) { + release(left); // the page may have moved to another document (a no-op while any host is still on `left`) + } } } return client; @@ -166,7 +193,9 @@ async function openWindow( } throw error; } finally { - flushQueuedAstroHydrationErrors(hydration); // success: report what queued; failure: end the window, nothing to report to + if (listened) { + flushQueuedAstroHydrationErrors(hydration); // success: report what queued; failure: end the window, nothing to report to + } } } From 83d626fa04d24d784ae9c351c1b2e56a1d0735d3 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 20:26:15 +0500 Subject: [PATCH 7/7] fix(astro): hold the launch window on a shared listener instead of skipping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skipping listen for a document another host's client uses also skipped the queueing window, so a relaunch of the host that owns that listener dropped island errors during the restart. The window is now held (pending+1, options untouched) and the new options are merged once the launch has succeeded; a failure still leaves the live listener's routing alone. Adds holdAstroHydrationWindow and updateAstroHydrationOptions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/astro/src/client.integration.test.ts | 25 ++++++++- packages/astro/src/client.test.ts | 34 +++++++++--- packages/astro/src/client.ts | 22 ++++---- packages/astro/src/hydration.test.ts | 53 +++++++++++++++++++ packages/astro/src/hydration.ts | 23 ++++++++ 5 files changed, 139 insertions(+), 18 deletions(-) diff --git a/packages/astro/src/client.integration.test.ts b/packages/astro/src/client.integration.test.ts index 7eb26c31..5fa3fe1b 100644 --- a/packages/astro/src/client.integration.test.ts +++ b/packages/astro/src/client.integration.test.ts @@ -5,7 +5,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // record on the real carrier the way core does. This is what the unit tests (mocked listen/stop) cannot see: which documents really // still have a listener, and where an error really goes. type Opts = Record & { carrier?: object }; -const state = { failNext: undefined as Error | undefined }; +const state = { + failNext: undefined as Error | undefined, + gate: undefined as Promise | undefined, +}; const clients: Array<{ logException: ReturnType }> = []; vi.mock('@bugsee/bugsee', () => { @@ -15,6 +18,7 @@ vi.mock('@bugsee/bugsee', () => { if (existing !== undefined) { return existing; } + await state.gate; if (state.failNext !== undefined) { const failure = state.failNext; state.failNext = undefined; @@ -59,6 +63,7 @@ describe('hydration listeners against the real registry', () => { delete (globalThis as { document?: unknown }).document; clients.length = 0; state.failNext = undefined; + state.gate = undefined; }); const reported = (client: { logException: ReturnType }, error: unknown) => @@ -171,4 +176,22 @@ describe('hydration listeners against the real registry', () => { docB.dispatchEvent(hydrationError(fresh)); expect(reported(client, fresh)).toBe(true); }); + + it("a relaunch of the host that owns a shared document's listener still queues errors during the restart", async () => { + const carrierA = {}; + const carrierB = {}; + const shared = page(); + await registerClient('tok', { carrier: carrierB, document: shared } as never); + await registerClient('tok', { carrier: carrierA, document: shared } as never); // A merged last: the listener routes to A + let open: () => void = () => undefined; + state.gate = new Promise((resolve) => (open = resolve)); + const relaunching = relaunch({ carrier: carrierA } as never); // A is stopped; its new client is not up yet + await new Promise((resolve) => setTimeout(resolve, 0)); + const during = new Error('island, mid-restart'); + shared.dispatchEvent(hydrationError(during)); + state.gate = undefined; + open(); + const restarted = (await relaunching) as unknown as (typeof clients)[number]; + expect(reported(restarted, during)).toBe(true); // queued, then reported to the new client: not dropped + }); }); diff --git a/packages/astro/src/client.test.ts b/packages/astro/src/client.test.ts index 668e42a0..23249daa 100644 --- a/packages/astro/src/client.test.ts +++ b/packages/astro/src/client.test.ts @@ -79,10 +79,12 @@ const world = vi.hoisted(() => { listen: vi.fn(), flush: vi.fn(), stop: vi.fn(), + hold: vi.fn(() => true), + update: vi.fn(), order: [] as string[], }; }); -const { launch, relaunchUmbrella, listen, flush, stop, order } = world; +const { launch, relaunchUmbrella, listen, flush, stop, hold, update, order } = world; vi.mock('@bugsee/bugsee', () => ({ launch, relaunch: relaunchUmbrella })); vi.mock('@bugsee/core', () => ({ getCarrier: (host: object = globalThis) => world.slot(host), @@ -93,6 +95,8 @@ vi.mock('./hydration', () => ({ listenForAstroHydrationErrors: world.listen, flushQueuedAstroHydrationErrors: world.flush, stopListeningForAstroHydrationErrors: world.stop, + holdAstroHydrationWindow: world.hold, + updateAstroHydrationOptions: world.update, })); const doc = (id: string) => ({ id }) as unknown as Document; @@ -119,6 +123,9 @@ describe('registerClient', () => { listen.mockReset(); flush.mockReset(); stop.mockReset(); + hold.mockReset(); + hold.mockImplementation(() => true); + update.mockReset(); order.length = 0; }); @@ -527,7 +534,7 @@ describe('registerClient', () => { expect(stop).toHaveBeenLastCalledWith(doc('docA')); }); - it("a launch that fails on a document another host's live client listens on does not touch that listener", async () => { + it("a launch that fails on a document another host's live client listens on holds the window but never touches that listener's options", async () => { const a = {}; const b = {}; const shared = doc('shared'); @@ -539,11 +546,14 @@ describe('registerClient', () => { 'boom', ); expect(listen).not.toHaveBeenCalled(); // B's carrier and sink never reached the live listener - expect(flush).not.toHaveBeenCalled(); + expect(update).not.toHaveBeenCalled(); + expect(hold).toHaveBeenCalledTimes(1); + expect(hold).toHaveBeenLastCalledWith(shared); + expect(flush).toHaveBeenCalledTimes(1); // the held window is settled expect(stop).not.toHaveBeenCalled(); }); - it('the same launch, once it succeeds, listens on the shared document and settles that registration', async () => { + it('the same launch, once it succeeds, merges its options onto the shared listener and settles that window', async () => { const a = {}; const b = {}; const shared = doc('shared'); @@ -551,9 +561,21 @@ describe('registerClient', () => { listen.mockClear(); flush.mockClear(); await registerClient('tok', { carrier: b, document: shared } as never); - expect(listen).toHaveBeenCalledTimes(1); + expect(listen).not.toHaveBeenCalled(); + expect(hold).toHaveBeenCalledTimes(1); + expect(update).toHaveBeenCalledTimes(1); + expect(update).toHaveBeenLastCalledWith({ target: shared, carrier: b, onError: undefined }); expect(flush).toHaveBeenCalledTimes(1); - expect(lastListen()).toEqual({ target: shared, carrier: b }); + }); + + it('when nothing is held on a shared document after all, it listens as usual', async () => { + const shared = doc('shared'); + await registerClient('tok', { carrier: {}, document: shared } as never); + hold.mockReturnValueOnce(false); + listen.mockClear(); + await registerClient('tok', { carrier: {}, document: shared } as never); + expect(listen).toHaveBeenCalledTimes(1); + expect(update).not.toHaveBeenCalled(); }); it('a relaunch that changes the carrier AND the document releases the document the lookup host was on', async () => { diff --git a/packages/astro/src/client.ts b/packages/astro/src/client.ts index 4f846e3d..d0a9886a 100644 --- a/packages/astro/src/client.ts +++ b/packages/astro/src/client.ts @@ -13,8 +13,10 @@ import { import { getCarrier, getCarrierClient, getCarrierPendingLaunch } from '@bugsee/core'; import { flushQueuedAstroHydrationErrors, + holdAstroHydrationWindow, listenForAstroHydrationErrors, stopListeningForAstroHydrationErrors, + updateAstroHydrationOptions, } from './hydration'; /** Options for the Astro client (browser) composition — the batteries-included browser umbrella options. */ @@ -156,18 +158,18 @@ async function openWindow( ...(target !== undefined ? { target } : {}), }; // Astro swallows an island's hydration failure into an `astro:hydration-error` event; nothing else would see it. Listen BEFORE the - // launch resolves (an error dispatched meanwhile is queued) and flush once the client exists. Not so on a document another host's client - // already listens on: merging this launch's carrier and sink onto that listener now would misroute the live client's errors if this launch - // then failed, so it waits for the launch to succeed (the live client's own listener covers the meantime). - let listened = tracked === undefined || !usedByOther(tracked, host); - if (listened) { + // launch resolves (an error dispatched meanwhile is queued) and flush once the client exists. On a document another host's client already + // listens on, only the window is HELD: merging this launch's carrier and sink onto that listener now would misroute the live client's + // errors if this launch then failed, so they are merged once it has succeeded. + const held = + tracked !== undefined && usedByOther(tracked, host) && holdAstroHydrationWindow(tracked); + if (!held) { listenForAstroHydrationErrors(hydration); } try { const client = await start(); - if (!listened) { - listenForAstroHydrationErrors(hydration); - listened = true; + if (held) { + updateAstroHydrationOptions(hydration); } if (tracked !== undefined) { // The document this client is now on, under the host it now lives on: a relaunch with `carrier: undefined` moves it to the global @@ -193,9 +195,7 @@ async function openWindow( } throw error; } finally { - if (listened) { - flushQueuedAstroHydrationErrors(hydration); // success: report what queued; failure: end the window, nothing to report to - } + flushQueuedAstroHydrationErrors(hydration); // success: report what queued; failure: end the window, nothing to report to } } diff --git a/packages/astro/src/hydration.test.ts b/packages/astro/src/hydration.test.ts index d4b73b5d..bd3c0374 100644 --- a/packages/astro/src/hydration.test.ts +++ b/packages/astro/src/hydration.test.ts @@ -3,8 +3,10 @@ import { setCarrierClient } from '@bugsee/core'; import { describe, expect, it, vi } from 'vitest'; import { flushQueuedAstroHydrationErrors, + holdAstroHydrationWindow, listenForAstroHydrationErrors, stopListeningForAstroHydrationErrors, + updateAstroHydrationOptions, } from './hydration'; const clientWith = (logException = vi.fn(() => Promise.resolve(undefined))) => @@ -333,3 +335,54 @@ describe('stopListeningForAstroHydrationErrors', () => { expect(logException).toHaveBeenCalledTimes(1); }); }); + +describe('holding a launch window on a listener without merging options', () => { + it('reports false when nothing listens on the target', () => { + expect(holdAstroHydrationWindow(new EventTarget())).toBe(false); + }); + + it('opens the queueing window while the listener keeps its own options, and merges them later without opening another', () => { + const target = new EventTarget(); + const first = vi.fn(() => Promise.resolve(undefined)); + const second = vi.fn(() => Promise.resolve(undefined)); + let current: Bugsee | undefined; // the first host's client: stopped for a relaunch + listenForAstroHydrationErrors({ target, getClient: () => current }); + flushQueuedAstroHydrationErrors({ target }); // the first registration settled: the window is closed + const early = new Error('dropped'); + target.dispatchEvent(hydrationError(early)); // no window, no client: dropped + expect(holdAstroHydrationWindow(target)).toBe(true); // a relaunch of that host begins + const during = new Error('during the restart'); + target.dispatchEvent(hydrationError(during)); // queued, and the listener still looks where it did + current = clientWith(first); + const afterRestart = { target, getClient: () => clientWith(second) }; + updateAstroHydrationOptions(afterRestart); // success: the new routing + flushQueuedAstroHydrationErrors(afterRestart); + expect(second).toHaveBeenCalledTimes(1); + expect((second.mock.calls[0] as unknown[])[0]).toBe(during); + expect(first).not.toHaveBeenCalled(); + target.dispatchEvent(hydrationError(new Error('later'))); + expect(second).toHaveBeenCalledTimes(2); // routed to the merged options + // the hold was settled by that one flush: the window is closed again, so a later error with no client is dropped + expect(holdAstroHydrationWindow(new EventTarget())).toBe(false); + }); + + it('a held window that is settled WITHOUT merging leaves the routing untouched', () => { + const target = new EventTarget(); + const owner = vi.fn(() => Promise.resolve(undefined)); + const options = { target, getClient: () => clientWith(owner) }; + listenForAstroHydrationErrors(options); + flushQueuedAstroHydrationErrors(options); + expect(holdAstroHydrationWindow(target)).toBe(true); + flushQueuedAstroHydrationErrors({ target, getClient: () => undefined }); // a failed sibling launch: settle only + const error = new Error('island'); + target.dispatchEvent(hydrationError(error)); + expect(owner).toHaveBeenCalledTimes(1); + expect((owner.mock.calls[0] as unknown[])[0]).toBe(error); + }); + + it('updating options for a target nobody listens on is a no-op', () => { + expect(() => + updateAstroHydrationOptions({ target: new EventTarget(), getClient: () => undefined }), + ).not.toThrow(); + }); +}); diff --git a/packages/astro/src/hydration.ts b/packages/astro/src/hydration.ts index 2780aac8..14495d1c 100644 --- a/packages/astro/src/hydration.ts +++ b/packages/astro/src/hydration.ts @@ -114,6 +114,29 @@ export function listenForAstroHydrationErrors(options: AstroHydrationOptions = { }; } +/** + * Open a launch window on a listener that already exists, WITHOUT touching its options: a registration that must not change where errors go + * until its launch has succeeded (another host's client uses this listener). Settle it with `flushQueuedAstroHydrationErrors`. False when + * nothing listens on `target`. + */ +export function holdAstroHydrationWindow(target: EventTarget): boolean { + const state = listening.get(target); + if (state === undefined) { + return false; + } + state.pending += 1; + return true; +} + +/** Merge `options` onto the listener of their target without opening a window (the counterpart of `holdAstroHydrationWindow`). */ +export function updateAstroHydrationOptions(options: AstroHydrationOptions): void { + const target = targetOf(options); + const state = target === undefined ? undefined : listening.get(target); + if (state !== undefined) { + state.options = { ...state.options, ...options }; + } +} + /** * Stop listening on `target` (the page moved to another document): removes the listener and forgets its queue and options. A no-op for a * target that is not being listened on.