diff --git a/config/scripts/mobile-web-app-terminal-render-fixture.mjs b/config/scripts/mobile-web-app-terminal-render-fixture.mjs index 7e92e68c5c4..375f8bb06ce 100644 --- a/config/scripts/mobile-web-app-terminal-render-fixture.mjs +++ b/config/scripts/mobile-web-app-terminal-render-fixture.mjs @@ -47,37 +47,69 @@ const SHELL_HOST = { lastConnected: 1 } -/** Builds the bundle, serves it under the shell's own policy, and launches the browser. */ +/** + * Everything the fixture allocated, in the reverse of the order it took it. + * + * Shared by the normal close and the rollback, because a setup that fell over halfway has exactly + * the same things to give back as one that ran to the end — it just has fewer of them. The server + * close is awaited rather than fired: it holds a listening socket, and a socket still open when + * the file finishes keeps the vitest worker alive after its last test has reported. + */ +async function closeTerminalRenderFixture({ browser, scratch, server }) { + await browser?.close() + if (server) { + await new Promise((resolve) => server.close(resolve)) + } + await rm(scratch, { recursive: true, force: true }) +} + +/** + * Builds the bundle, serves it under the shell's own policy, and launches the browser. + * + * Nothing survives a setup that throws. The browser is launched last and is the step most likely + * to fail — no Chromium on the machine, an `ORCA_MOBILE_WEB_RENDER_BROWSER` that points nowhere — + * and by then the server is listening and the scratch tree is on disk. A caller that never got a + * handle back has nothing to close, so this closes them itself and rethrows what actually went + * wrong rather than whatever the cleanup might say. + */ export async function startTerminalRenderFixture() { const cspHeader = await readShellCsp() const bridgeVersion = await readBridgeProtocolVersion() const faultGrant = await readBridgeFaultGrant() const scratch = await mkdtemp(join(tmpdir(), 'orca-c75-terminal-render-')) - const appDir = join(scratch, 'app') - const routeDir = join(appDir, MOBILE_WEB_APP_ROUTE_ROOT) - await mkdir(routeDir, { recursive: true }) - await writeFile(join(routeDir, '_layout.tsx'), LAYOUT_SOURCE) - // Extensionless, so the bundler resolves the `.web.tsx` sibling exactly as it would for a real - // route. Naming the `.tsx` would mount the WebView wrapper no browser can render. - await writeFile( - join(routeDir, 'terminal-probe.tsx'), - probeRouteSource(join(mobileDir, 'src', 'terminal', 'TerminalWebView')) - ) - await writeFile(join(routeDir, 'terminal-control.tsx'), CONTROL_SOURCE) - const built = await buildMobileWebAppBundle({ - appDir, - outDir: join(scratch, 'bundle'), - pageRoutes: [ - { pathname: PROBE_ROUTE, grants: [] }, - { pathname: CONTROL_ROUTE, grants: [] } - ] - }) - const { origin, server } = await createBundleServer({ outDir: built.outDir, cspHeader }) - const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER - const browser = await chromium.launch({ - headless: true, - ...(executablePath ? { executablePath } : {}) - }) + let browser = null + let served = null + try { + const appDir = join(scratch, 'app') + const routeDir = join(appDir, MOBILE_WEB_APP_ROUTE_ROOT) + await mkdir(routeDir, { recursive: true }) + await writeFile(join(routeDir, '_layout.tsx'), LAYOUT_SOURCE) + // Extensionless, so the bundler resolves the `.web.tsx` sibling exactly as it would for a + // real route. Naming the `.tsx` would mount the WebView wrapper no browser can render. + await writeFile( + join(routeDir, 'terminal-probe.tsx'), + probeRouteSource(join(mobileDir, 'src', 'terminal', 'TerminalWebView')) + ) + await writeFile(join(routeDir, 'terminal-control.tsx'), CONTROL_SOURCE) + const built = await buildMobileWebAppBundle({ + appDir, + outDir: join(scratch, 'bundle'), + pageRoutes: [ + { pathname: PROBE_ROUTE, grants: [] }, + { pathname: CONTROL_ROUTE, grants: [] } + ] + }) + served = await createBundleServer({ outDir: built.outDir, cspHeader }) + const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER + browser = await chromium.launch({ + headless: true, + ...(executablePath ? { executablePath } : {}) + }) + } catch (error) { + await closeTerminalRenderFixture({ browser, scratch, server: served?.server }) + throw error + } + const { origin } = served async function openPage( pathname, @@ -134,11 +166,7 @@ export async function startTerminalRenderFixture() { return { openPage, openTerminal, - close: async () => { - await browser.close() - server.close() - await rm(scratch, { recursive: true, force: true }) - } + close: () => closeTerminalRenderFixture({ browser, scratch, server: served.server }) } } diff --git a/config/scripts/mobile-web-app-terminal-render-fixture.test.mjs b/config/scripts/mobile-web-app-terminal-render-fixture.test.mjs new file mode 100644 index 00000000000..477f838460e --- /dev/null +++ b/config/scripts/mobile-web-app-terminal-render-fixture.test.mjs @@ -0,0 +1,85 @@ +import { readdir } from 'node:fs/promises' +import { connect } from 'node:net' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' + +/** + * What the render fixture gives back when it never finishes starting. + * + * The handle is the only way to close it, so a setup that throws before returning one leaves the + * caller nothing to call: `afterAll` has no fixture, and the listening socket and the scratch tree + * stay where they are. The socket is the part that bites — an open server handle keeps the vitest + * worker alive after its last test has reported, so the file hangs rather than failing. + * + * The browser is the step that fails in practice and the last one taken, so by then everything + * else is allocated. It is made to fail the way it actually does, by pointing the launch at an + * executable that is not there, rather than by standing a double in front of Playwright. + */ + +const SCRATCH_PREFIX = 'orca-c75-terminal-render-' +const describeFixture = mobileWebAppDependenciesPresent() ? describe : describe.skip + +// The port the fixture served on, which it never hands out and which `close` makes unreachable. +// Recorded through the real server rather than a double: what is under test is whether the thing +// that was actually listening stopped. +const { ports } = vi.hoisted(() => ({ ports: [] })) +vi.mock('./mobile-web-app-render-harness.mjs', async (importOriginal) => { + const harness = await importOriginal() + return { + ...harness, + createBundleServer: async (options) => { + const served = await harness.createBundleServer(options) + ports.push(served.server.address().port) + return served + } + } +}) + +const { startTerminalRenderFixture } = await import('./mobile-web-app-terminal-render-fixture.mjs') + +/** Whether a connection to this port is refused, which is what a closed listener answers. */ +function refusesConnections(port) { + return new Promise((resolve) => { + const socket = connect({ host: '127.0.0.1', port }) + socket.on('connect', () => { + socket.destroy() + resolve(false) + }) + socket.on('error', () => resolve(true)) + }) +} + +async function scratchDirectories() { + const entries = await readdir(tmpdir()) + return entries.filter((entry) => entry.startsWith(SCRATCH_PREFIX)).sort() +} + +describeFixture('the terminal render fixture', () => { + it('takes back the server and the scratch tree when the browser will not start', async () => { + const scratchBefore = await scratchDirectories() + const realBrowser = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER + process.env.ORCA_MOBILE_WEB_RENDER_BROWSER = join(tmpdir(), 'orca-c75-no-such-browser') + try { + // Named, not merely thrown: a build that broke for its own reason would also reject, and + // would satisfy a bare `toThrow` while saying nothing about the rollback under test. It + // also has to be the original error and not whatever the cleanup raised on its way out. + await expect(startTerminalRenderFixture()).rejects.toThrow( + /Failed to launch chromium because executable doesn't exist/ + ) + } finally { + if (realBrowser === undefined) { + delete process.env.ORCA_MOBILE_WEB_RENDER_BROWSER + } else { + process.env.ORCA_MOBILE_WEB_RENDER_BROWSER = realBrowser + } + } + + // The precondition: the launch has to have been reached with a server already listening, or + // there is nothing for the rollback to have released and the refusal below means nothing. + expect(ports, 'the setup got as far as serving the bundle').toHaveLength(1) + expect(await refusesConnections(ports[0])).toBe(true) + expect(await scratchDirectories()).toEqual(scratchBefore) + }, 600_000) +}) diff --git a/mobile/src/terminal/terminal-web-document-mount.ts b/mobile/src/terminal/terminal-web-document-mount.ts index bd80e3b06ae..8f29a4abc01 100644 --- a/mobile/src/terminal/terminal-web-document-mount.ts +++ b/mobile/src/terminal/terminal-web-document-mount.ts @@ -137,10 +137,19 @@ export function mountTerminalWebDocument( const token = Symbol('orca terminal document') liveDocument = token let started: StartedDocument | null = null + /** + * Gives the page back, and only if it is still this mount's to give. + * + * The check covers the element too, not just the claim. Emptying a host and taking its class + * off are what make the terminal disappear, so a release that skipped the claim but did those + * anyway would blank the terminal a later mount has on the screen. One rule, inside the thing + * it governs, rather than at each caller. + */ const release = () => { - if (liveDocument === token) { - liveDocument = null + if (liveDocument !== token) { + return } + liveDocument = null host.innerHTML = '' // The sheet stays in the head; the class does not, so every rule in it matches nothing // again the moment the terminal is gone. @@ -171,12 +180,9 @@ export function mountTerminalWebDocument( }, (error: unknown) => { // The import failed, so nothing was started and the page has to go back — the overlay's - // Reload is a second mount and it must be allowed to make one. Only if the page is still - // this mount's: a later mount may already hold it, and emptying its host would take the - // terminal that is on the screen. - if (liveDocument === token) { - release() - } + // Reload is a second mount and it must be allowed to make one. A later mount may already + // hold the page, which `release` answers for. + release() throw error } )