fix(browser-relay): silent reannounce failure + extend nav re-attach window#27630
Conversation
Greptile SummaryFixes two connection reliability bugs in the browser relay extension that caused confusing "appears connected but broken" states. Bug 1 - Silent reannounce failure (fixed): Split try-catch blocks in Bug 2 - Navigation re-attach window too short (fixed): Extended retry delays from Confidence Score: 4/5
Last reviewed commit: ae874da |
| let targetInfo | ||
| try { | ||
| const info = /** @type {any} */ ( | ||
| await chrome.debugger.sendCommand({ tabId }, 'Target.getTargetInfo') | ||
| ) | ||
| const targetInfo = info?.targetInfo | ||
| targetInfo = info?.targetInfo | ||
| } catch { | ||
| // Target.getTargetInfo failed — debugger lost the tab info. | ||
| // Tab state remains valid (Runtime.evaluate succeeded above); proceed | ||
| // with whatever targetInfo we have cached and let the relay use it. | ||
| } |
There was a problem hiding this comment.
when Target.getTargetInfo fails, targetInfo is undefined, so the relay receives { attached: true } without targetId. consider using the cached tab.targetId as a fallback:
| let targetInfo | |
| try { | |
| const info = /** @type {any} */ ( | |
| await chrome.debugger.sendCommand({ tabId }, 'Target.getTargetInfo') | |
| ) | |
| const targetInfo = info?.targetInfo | |
| targetInfo = info?.targetInfo | |
| } catch { | |
| // Target.getTargetInfo failed — debugger lost the tab info. | |
| // Tab state remains valid (Runtime.evaluate succeeded above); proceed | |
| // with whatever targetInfo we have cached and let the relay use it. | |
| } | |
| let targetInfo | |
| try { | |
| const info = /** @type {any} */ ( | |
| await chrome.debugger.sendCommand({ tabId }, 'Target.getTargetInfo') | |
| ) | |
| targetInfo = info?.targetInfo | |
| } catch { | |
| // Target.getTargetInfo failed — use cached targetId as fallback so relay | |
| // receives at least the core identifier rather than just { attached: true }. | |
| targetInfo = tab.targetId ? { targetId: tab.targetId } : undefined | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: assets/chrome-extension/background.js
Line: 285-295
Comment:
when `Target.getTargetInfo` fails, `targetInfo` is `undefined`, so the relay receives `{ attached: true }` without `targetId`. consider using the cached `tab.targetId` as a fallback:
```suggestion
let targetInfo
try {
const info = /** @type {any} */ (
await chrome.debugger.sendCommand({ tabId }, 'Target.getTargetInfo')
)
targetInfo = info?.targetInfo
} catch {
// Target.getTargetInfo failed — use cached targetId as fallback so relay
// receives at least the core identifier rather than just { attached: true }.
targetInfo = tab.targetId ? { targetId: tab.targetId } : undefined
}
```
How can I resolve this? If you propose a fix, please make it concise.|
Pushed follow-up fixes for the Windows CI failures on this PR head (
Files touched:
Local verification on this exact SHA:
I also re-ran targeted suites for the prior failures ( I currently only see |
ce736c8 to
8973e21
Compare
8973e21 to
d6de1bc
Compare
|
Landed via temp rebase onto main.
Checking formatting... All matched files use the correct format.
Found 0 warnings and 0 errors.
OK apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift
A2UI bundle up to date; skipping.
[copy-hook-metadata] Copied boot-md/HOOK.md
RUN v4.0.18 /Users/steipete/Projects/clawdbot3 ✓ src/cli/program/preaction.test.ts (10 tests) 890ms Test Files 17 passed (17) ✓ src/infra/process-respawn.test.ts (10 tests) 14ms Test Files 1 failed | 717 passed | 1 skipped (719) ELIFECYCLE Test failed. See above for more details. Thanks @markmusson! |
Problem
Two bugs in
assets/chrome-extension/background.jsthat cause the relay to appear connected while broken — the "it keeps breaking" complaint from users.Bug 1: Silent reannounce failure
In
reannounceAttachedTabs(),sendToRelay()was inside the sametry-catchblock asTarget.getTargetInfo. WhensendToRelaythrew — which happens when the relay WebSocket closes in the gap betweenensureRelayConnectionresolving and the loop executing — the catch set the badge to'on'regardless.Result: relay has
extensionConnected=truebutconnectedTargetsis empty. Every browser tool call fails with "no tab connected" but the badge shows ON. No visible signal to the user. The broken state persists until the next full reconnect cycle.Fix: Split into two try-catch blocks. Debugger failures proceed with cached targetInfo. Relay send failures set badge to
'connecting'(not'on'), so the keepalive/reconnect cycle retries and the user sees the real state.Bug 2: Navigation re-attach window too short
delays = [300, 700, 1500]— 3 attempts, 2.5 s total. SPAs and pages with heavy JS (React, Next.js, Vite apps) can take longer than 2.5 s before the Chrome debugger is attachable after navigation. All three attempts fail, badge goes permanently off, user has to re-click after every page load.Fix:
[200, 500, 1000, 2000, 4000]— 5 attempts, ~7.7 s total.Testing
Reproduces the issue reported in https://x.com/rohan1chau/status/[tweet]