Skip to content

Commit 6d6dfee

Browse files
authored
A bridge browser launch that cannot begin is reported, not a daemon crash (#1332) (#1723)
1 parent 5f26cea commit 6d6dfee

9 files changed

Lines changed: 44 additions & 13 deletions

packages/framework/dashboard/components/BridgeBrowserSettings.SPEC.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ The settings line for the bridge browser: where the daemon's own browser stands,
1212
- **A launch names its step** - so a minutes-long download does not read as a hang.
1313
- **A running browser can be shown and hidden** - shown for the sign-in, hidden again after; the button offered is the one that applies.
1414
- **The sign-in is asked for by name** - when the browser's Driver tab reports claude.ai's sign-in page, the line says to show the window and sign in once.
15-
- **A stopped browser says why, and offers a restart** - the reason the daemon recorded, and a Restart button.
15+
- **A browser that is not running says why, and offers a restart** - the reason the daemon recorded — an exit, a failed launch step, or a launch that could not begin — and a Restart button.
1616

1717
## Business logic
1818

@@ -56,15 +56,15 @@ The bridge browser is running but signed out, so nothing it does reaches a sessi
5656

5757
When the daemon reports the browser running and its claude.ai tab on the sign-in page, the line says so and to show the window, sign in once, then hide it again; the show button is worded for the sign-in. Once the tab is on any other page, the prompt is gone. The daemon reads that page from its own browser, so the user's own Chrome reporting to the bridge at the same time cannot mask it.
5858

59-
### A stopped browser says why, and offers a restart
59+
### A browser that is not running says why, and offers a restart
6060

6161
#### User story
6262

6363
See `## User story`: the browser was quit, or could not start.
6464

6565
#### Business logic
6666

67-
When the daemon reports the browser as stopped, the line quotes the reason — the exit signal, or the launch step that failed — and offers a Restart, which asks the daemon to launch it again.
67+
When the daemon reports the browser as stopped, the line says it is not running and quotes the reason — the exit signal, the launch step that failed, or why the launch could not begin (the bridge token the daemon reads at start-up is missing, so a restart is needed) — and offers a Restart, which asks the daemon to launch it again.
6868

6969
## Before modifying/creating SPEC.md files
7070

packages/framework/dashboard/components/BridgeBrowserSettings.test.SPEC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ What the bridge browser settings line's tests cover, against faked daemon reads
33
- **A launch under way** - the line quotes the daemon's current step, download percentage included.
44
- **The sign-in page** - a running, minimized browser whose Driver tab reported claude.ai's sign-in page gets the sign-in prompt and a show button worded for it; pressing it asks the daemon to show the window.
55
- **A shown window** - offers to hide it, asking the daemon to; a Driver tab on any other page gets no sign-in prompt.
6-
- **A stopped browser** - the reason is quoted and Restart asks the daemon to launch again.
6+
- **A browser that is not running** - the reason is quoted and Restart asks the daemon to launch again.
77
- **Switched off** - nothing is rendered and nothing is read.
88

99
## Before modifying/creating SPEC.md files

packages/framework/dashboard/components/BridgeBrowserSettings.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('BridgeBrowserSettings (#1332)', () => {
4343
test('a browser that stopped says why and offers a restart, because quitting it was an act', async () => {
4444
onBridgeBrowser.mockResolvedValue({ state: 'stopped', detail: 'Chrome exited on SIGTERM' })
4545
render(<BridgeBrowserSettings enabled />)
46-
const line = await screen.findByText(/The bridge browser stopped/)
46+
const line = await screen.findByText(/The bridge browser is not running/)
4747
expect(line.textContent).toContain('Chrome exited on SIGTERM')
4848
fireEvent.click(screen.getByText('Restart'))
4949
expect(sendBridgeBrowser).toHaveBeenCalledWith('restart')

packages/framework/dashboard/components/BridgeBrowserSettings.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function BridgeBrowserSettings({ enabled }: { enabled: boolean }) {
4848
{status.state === 'starting' && <p className="text-muted-foreground">Starting the bridge browser: {status.detail}.</p>}
4949
{status.state === 'stopped' && (
5050
<p className="text-danger">
51-
The bridge browser stopped: {status.detail}.{' '}
51+
The bridge browser is not running: {status.detail}.{' '}
5252
<Button variant="outline" size="sm" onClick={() => act('restart')}>
5353
Restart
5454
</Button>

packages/framework/src/bridge-browser.SPEC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ See `## User story`: cloud sessions are served whether or not the user's Chrome
3232

3333
#### Business logic
3434

35-
The daemon keeps at most one bridge browser. It launches it when the bridge browser preference is on and the dashboard is listening — the extension must be told the daemon's address — and only while the browser bridge itself is on, since without the bridge token there is nothing to hand the extension; with the bridge off, the launch fails saying to turn the bridge on and restart the dashboard. Switching the preference on launches the browser at once; switching it off closes it; stopping the daemon closes it. Asking for a launch while one is under way or a browser is running does nothing more. A stop that lands while a launch is under way closes the browser that launch then hands over, so a browser is never left running unowned.
35+
The daemon keeps at most one bridge browser. It launches it when the bridge browser preference is on and the dashboard is listening — the extension must be told the daemon's address — and only while the browser bridge itself is on, since without the bridge token there is nothing to hand the extension; the bridge token is read when the daemon starts, so a launch asked for without one fails saying what to do — restart the dashboard when the bridge was switched on after it started, turn the bridge on first when it is off. A launch that fails before it begins is reported exactly like one that fails on the way: as stopped, with the reason, never as an error that escapes the daemon. Switching the preference on launches the browser at once; switching it off closes it; stopping the daemon closes it. Asking for a launch while one is under way or a browser is running does nothing more. A stop that lands while a launch is under way closes the browser that launch then hands over, so a browser is never left running unowned.
3636

3737
The dashboard reads where the browser stands: off; starting, with the step it is on; running, whether its window is shown, and whether its claude.ai tab is on the sign-in page (or the sign-out step that leads there) — read from the browser itself, not from the bridge, whose last report may be the user's own Chrome's; or stopped, with the reason. It can ask for the window to be shown or hidden, or for a restart (a stop, then a launch).
3838

packages/framework/src/bridge-browser.test.SPEC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ What the bridge browser's tests cover. Every launch runs against a fake Chrome (
1212
- **A failed set-up step** - the browser is asked to close and killed when it does not, and the launch fails naming the step.
1313
- **A worker that refuses the token** - fails the launch quoting the worker's answer.
1414
- **An exit on its own** - reported once with the signal; an exit the daemon's own close caused is not reported, and a browser already gone is not killed again.
15-
- **The owner** - reports the launch's step, then running (and whether shown), then stopped with the reason when the browser exits; show and hide do nothing once it is gone. A failed launch is stopped with the reason and a later start tries again. A stop during a launch closes the browser the launch hands over. Repeated starts are one launch; restart is a stop then a start.
15+
- **The owner** - reports the launch's step, then running (and whether shown), then stopped with the reason when the browser exits; show and hide do nothing once it is gone. A failed launch is stopped with the reason and a later start tries again — including a launch that fails before it begins, when the launcher throws outright. A stop during a launch closes the browser the launch hands over. Repeated starts are one launch; restart is a stop then a start.
1616

1717
## Before modifying/creating SPEC.md files
1818

packages/framework/src/bridge-browser.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,19 @@ test('a failed launch is stopped with the reason; a second start tries again', a
309309
assert.equal((await owner.status()).state, 'running')
310310
})
311311

312+
test('a launcher that throws before launching is a failed launch with the reason, not a crash (#1332)', async () => {
313+
const lines: string[] = []
314+
const owner = bridgeBrowserOwner(
315+
() => {
316+
throw new Error('turn the browser bridge on, then restart the dashboard')
317+
},
318+
line => lines.push(line),
319+
)
320+
await owner.start()
321+
assert.deepEqual(await owner.status(), { state: 'stopped', detail: 'turn the browser bridge on, then restart the dashboard' })
322+
assert.ok(lines.some(line => line.includes('could not start')))
323+
})
324+
312325
test('a stop during a launch closes the browser the launch then hands over', async () => {
313326
const launch = controlledLaunch()
314327
const owner = bridgeBrowserOwner(launch.launch)

packages/framework/src/bridge-browser.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -524,9 +524,18 @@ export function bridgeBrowserOwner(launch: BridgeBrowserLauncher, log: (line: st
524524
if (browser || launching) return launching
525525
const mine = ++generation
526526
current = { state: 'starting', detail: 'preparing' }
527-
launching = launch(detail => {
528-
if (generation === mine) current = { state: 'starting', detail }
529-
})
527+
// A launcher that throws before it has anything to await — the bridge token it needs does not
528+
// exist — is a launch that failed, and goes the same way as one that failed later. Left as a
529+
// plain call, that throw would escape `start()` as a rejection nobody handles, and an
530+
// unhandled rejection ends the daemon process (#1332). The executor runs at once, so the
531+
// launcher is still called synchronously and its first step is readable right away.
532+
launching = new Promise<BridgeBrowser>(resolve =>
533+
resolve(
534+
launch(detail => {
535+
if (generation === mine) current = { state: 'starting', detail }
536+
}),
537+
),
538+
)
530539
.then(async launched => {
531540
if (generation !== mine) {
532541
await launched.close()

packages/framework/src/daemon.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,17 @@ export async function runDaemon(cwd: string, opts: RunDaemonOptions = {}): Promi
192192
// so a web run no longer needs the user's Chrome open. Created here, launched only once the
193193
// dashboard listens (the extension is told the daemon's address) and the preference says so.
194194
// It needs the bridge: without the token there is nothing to hand the extension.
195-
const bridgeBrowser = bridgeBrowserOwner(report => {
196-
if (!bridgeToken || !daemonUrl) throw new Error('turn the browser bridge on and restart the dashboard first')
195+
const bridgeBrowser = bridgeBrowserOwner(async report => {
196+
if (!bridgeToken || !daemonUrl) {
197+
// The token is minted at boot, from the bridge preference as it stood then (above). A bridge
198+
// switched on since is a restart away; a bridge that is off wants switching on first.
199+
const bridgeOnNow = (await readPreferences(undefined, env).catch((): Preferences => ({}))).bridge === true
200+
throw new Error(
201+
bridgeOnNow
202+
? 'the browser bridge was switched on after the dashboard started — restart the dashboard, and the browser launches on its own'
203+
: 'turn the browser bridge on, then restart the dashboard',
204+
)
205+
}
197206
return (opts.bridgeBrowser ?? (o => startBridgeBrowser(o)))({ daemonUrl, token: bridgeToken, dir: bridgeBrowserDir(env), report })
198207
}, console.log)
199208
// Assigned below, read from the credentials store's `onChange` (#1095): the dashboard mount has

0 commit comments

Comments
 (0)