From 81032027c620ef12d21e0476bdfcbbfeff4260e0 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:50:38 +0100 Subject: [PATCH] fix: try a plugin once more when a busy machine makes it miss its start limit --- lib/__tests__/plugin-sandbox-loader.test.ts | 75 +++++++++++++++++++++ lib/plugin-sandbox/loader.ts | 28 ++++++-- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/lib/__tests__/plugin-sandbox-loader.test.ts b/lib/__tests__/plugin-sandbox-loader.test.ts index ddb2fc49b..24efba369 100644 --- a/lib/__tests__/plugin-sandbox-loader.test.ts +++ b/lib/__tests__/plugin-sandbox-loader.test.ts @@ -121,6 +121,81 @@ describe('loadSandboxedPlugin in the no-third-party mode (#903)', () => { }); }); +describe('loadSandboxedPlugin when a frame is slow to boot', () => { + const hung = () => ({ ...fakeBackground(), initPromise: new Promise(() => {}) }); + + // The bundle check runs on real promises and crypto: wait for the frame + // that is due before moving the clock past its timeout. + const framesStarted = async (count: number) => { + for (let i = 0; i < 2000 && mocks.createBackgroundInstance.mock.calls.length < count; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } + await new Promise((resolve) => setImmediate(resolve)); + }; + + beforeEach(() => { vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); }); + afterEach(() => { vi.useRealTimers(); }); + + it('tries once more after the init timeout and runs when the second frame answers', async () => { + mocks.getCode.mockResolvedValue(CODE); + const first = hung(); + mocks.createBackgroundInstance.mockReturnValueOnce(first); + + const run = loadSandboxedPlugin(plugin()); + await framesStarted(1); + await vi.advanceTimersByTimeAsync(30_000); + await framesStarted(2); + await run; + + expect(first.destroy).toHaveBeenCalled(); + expect(mocks.createBackgroundInstance).toHaveBeenCalledTimes(2); + expect(statuses.some((s) => s.status === 'error')).toBe(false); + expect(lastStatus()).toMatchObject({ id: 'managed-plugin', status: 'running' }); + }); + + it('reports the error when the second try times out too, and stops there', async () => { + mocks.getCode.mockResolvedValue(CODE); + mocks.createBackgroundInstance.mockImplementation(() => hung()); + + const run = loadSandboxedPlugin(plugin()); + await framesStarted(1); + await vi.advanceTimersByTimeAsync(30_000); + await framesStarted(2); + await vi.advanceTimersByTimeAsync(30_000); + await run; + + expect(mocks.createBackgroundInstance).toHaveBeenCalledTimes(2); + expect(lastStatus()).toMatchObject({ id: 'managed-plugin', status: 'error' }); + expect(lastStatus().error).toMatch(/timed out/); + }); + + it('does not try again when the plugin was turned off while it started', async () => { + mocks.getCode.mockResolvedValue(CODE); + mocks.createBackgroundInstance.mockImplementation(() => hung()); + + const run = loadSandboxedPlugin(plugin()); + await framesStarted(1); + unloadSandboxedPlugin('managed-plugin'); + await vi.advanceTimersByTimeAsync(30_000); + await run; + + expect(mocks.createBackgroundInstance).toHaveBeenCalledTimes(1); + }); + + it('does not retry a plugin that fails for another reason', async () => { + mocks.getCode.mockResolvedValue(CODE); + mocks.createBackgroundInstance.mockImplementationOnce(() => ({ + ...fakeBackground(), + initPromise: Promise.reject(new Error('bundle threw')), + })); + + await loadSandboxedPlugin(plugin()); + + expect(mocks.createBackgroundInstance).toHaveBeenCalledTimes(1); + expect(lastStatus()).toMatchObject({ status: 'error', error: 'bundle threw' }); + }); +}); + describe('loadSandboxedPlugin bundle sourcing', () => { it('runs a managed plugin from IndexedDB when the cached copy verifies', async () => { mocks.getCode.mockResolvedValue(CODE); diff --git a/lib/plugin-sandbox/loader.ts b/lib/plugin-sandbox/loader.ts index cff371fa0..226f541e0 100644 --- a/lib/plugin-sandbox/loader.ts +++ b/lib/plugin-sandbox/loader.ts @@ -197,10 +197,12 @@ function permittedOffers(plugin: InstalledPlugin, slots: InitDoneInfo['slots'], // slower machines, while still catching truly stuck plugins. const INIT_TIMEOUT_MS = 30_000; +class InitTimeoutError extends Error {} + function withTimeout(promise: Promise, ms: number, label: string): Promise { return new Promise((resolve, reject) => { const timer = setTimeout(() => { - reject(new Error(`${label} timed out after ${ms}ms`)); + reject(new InitTimeoutError(`${label} timed out after ${ms}ms`)); }, ms); promise.then( (v) => { clearTimeout(timer); resolve(v); }, @@ -210,6 +212,8 @@ function withTimeout(promise: Promise, ms: number, label: string): Promise } const loading = new Map>(); +/** Turned off while starting: a start that timed out doesn't try again. */ +const startsCancelled = new Set(); /** * Start a plugin. Calling it again for a plugin that is running or still @@ -221,14 +225,20 @@ export function loadSandboxedPlugin(plugin: InstalledPlugin): Promise { // Before the registry: a plugin offering remembered slots is registered // while it is still starting. const pending = loading.get(plugin.id); - if (pending) return pending; + if (pending) { + startsCancelled.delete(plugin.id); + return pending; + } if (getActive(plugin.id)) return Promise.resolve(); - const run = startSandboxedPlugin(plugin).finally(() => loading.delete(plugin.id)); + const run = startSandboxedPlugin(plugin).finally(() => { + loading.delete(plugin.id); + startsCancelled.delete(plugin.id); + }); loading.set(plugin.id, run); return run; } -async function startSandboxedPlugin(plugin: InstalledPlugin): Promise { +async function startSandboxedPlugin(plugin: InstalledPlugin, retried = false): Promise { // A plugin that declares network access doesn't start while third-party // traffic is off; the rest run (#903). if (!pluginAllowed(plugin)) { @@ -352,6 +362,15 @@ async function startSandboxedPlugin(plugin: InstalledPlugin): Promise { if (background?.closed && getActive(plugin.id)?.background !== background) return; // Offered from the last start: take the slots back down. if (background && getActive(plugin.id)?.background === background) deregisterActive(plugin.id); + // A busy machine can take longer than the limit to boot a frame, and the + // plugin then stayed off until the next page load: its pages and buttons + // missing. One more go; plugins start in parallel, so a plugin that + // really hangs holds up nobody else. + if (err instanceof InitTimeoutError && !retried && !startsCancelled.has(plugin.id)) { + console.warn(`[plugin-sandbox] "${plugin.id}" took too long to start, trying once more`); + try { background?.destroy(); } catch { /* ignore */ } + return startSandboxedPlugin(plugin, true); + } const msg = (err as Error).message ?? String(err); storeAccessor?.setPluginStatus(plugin.id, 'error', msg); console.error(`[plugin-sandbox] Failed to load "${plugin.id}":`, err); @@ -414,6 +433,7 @@ setCheckerDispatcher({ // ─── Unload ─────────────────────────────────────────────────── export function unloadSandboxedPlugin(pluginId: string): void { + if (loading.has(pluginId)) startsCancelled.add(pluginId); const entry = deregisterActive(pluginId); if (!entry) return; for (const d of entry.hookDisposables) {