fix(browser): surface CDP recovery errors#1736
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR implements graceful degradation for YoBrowser CDP failures when the browser panel becomes unavailable mid-session. It introduces a recoverable error type, detection logic in the tool handler, structured error transformation in the agent tool manager, and recovery instructions in the tool system prompt. ChangesYoBrowser CDP Graceful Degradation
Sequence Diagram(s)sequenceDiagram
participant Agent
participant AgentToolManager
participant YoBrowserHandler
participant Presenter
participant ErrorPayload
Agent->>AgentToolManager: callTool('cdp_send', args)
AgentToolManager->>YoBrowserHandler: callTool('cdp_send', args)
YoBrowserHandler->>Presenter: getBrowserStatus(sessionId)
Presenter-->>YoBrowserHandler: {initialized: false, page: null, ...}
alt Browser unavailable
YoBrowserHandler->>ErrorPayload: buildYoBrowserUnavailablePayload(sessionId, method, status)
ErrorPayload-->>YoBrowserHandler: {error: {code, recoverable, nextActions}, ...}
YoBrowserHandler->>YoBrowserHandler: throw YoBrowserUnavailableError(payload)
YoBrowserHandler-->>AgentToolManager: YoBrowserUnavailableError
AgentToolManager-->>AgentToolManager: rethrow (propagate upward)
else Browser ready
YoBrowserHandler->>Presenter: sendCdpCommand(...)
Presenter-->>YoBrowserHandler: result
YoBrowserHandler-->>AgentToolManager: success response
AgentToolManager-->>Agent: tool result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/issues/yobrowser-cdp-graceful-degradation/tasks.md (1)
16-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
typecheckto the validation checklist for consistency.The task list omits
pnpm run typecheck, which is part of this PR’s stated validation steps. Adding it keeps the checklist auditable.Suggested diff
- [x] Run `pnpm run format`. - [x] Run `pnpm run i18n`. - [x] Run `pnpm run lint`. +- [x] Run `pnpm run typecheck`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/issues/yobrowser-cdp-graceful-degradation/tasks.md` around lines 16 - 19, Add the missing checklist entry for typechecking in the tasks checklist by inserting an item for "Run `pnpm run typecheck`" alongside the existing format/i18n/lint lines in the tasks block (the checklist items in docs/issues/yobrowser-cdp-graceful-degradation/tasks.md), ensuring it matches the same checked-box style ("- [x] Run `pnpm run typecheck`.") so the validation steps are auditable and consistent.src/main/presenter/browser/YoBrowserToolHandler.ts (1)
67-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh
browserStatusbefore wrappingYoBrowserNotReadyError.Line 76 reuses the preflight
status, so if the page detaches after Line 51 the emittedpayload.error.browserStatuscan still say the session is initialized/visible. That makes the new recoverable error contract report the wrong state right when the agent needs it for recovery. Build this branch from a fresh status lookup instead of the stale snapshot.Proposed fix
- throw await this.createUnavailableError(sessionId, method, status, error) + throw await this.createUnavailableError(sessionId, method, undefined, error)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/presenter/browser/YoBrowserToolHandler.ts` around lines 67 - 77, The branch handling YoBrowserNotReadyError in YoBrowserToolHandler currently reuses the stale preflight status snapshot; before calling createUnavailableError (and before logging the final payload), refresh the page/browser status by invoking the same status lookup used earlier (e.g., the method or helper that produced `status` for the preflight check) to obtain an up-to-date `browserStatus`/page status, then use that fresh status in the logger and when calling this.createUnavailableError(sessionId, method, freshStatus, error) so the wrapped recoverable error reports current state rather than the stale snapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/issues/yobrowser-cdp-graceful-degradation/tasks.md`:
- Around line 16-19: Add the missing checklist entry for typechecking in the
tasks checklist by inserting an item for "Run `pnpm run typecheck`" alongside
the existing format/i18n/lint lines in the tasks block (the checklist items in
docs/issues/yobrowser-cdp-graceful-degradation/tasks.md), ensuring it matches
the same checked-box style ("- [x] Run `pnpm run typecheck`.") so the validation
steps are auditable and consistent.
In `@src/main/presenter/browser/YoBrowserToolHandler.ts`:
- Around line 67-77: The branch handling YoBrowserNotReadyError in
YoBrowserToolHandler currently reuses the stale preflight status snapshot;
before calling createUnavailableError (and before logging the final payload),
refresh the page/browser status by invoking the same status lookup used earlier
(e.g., the method or helper that produced `status` for the preflight check) to
obtain an up-to-date `browserStatus`/page status, then use that fresh status in
the logger and when calling this.createUnavailableError(sessionId, method,
freshStatus, error) so the wrapped recoverable error reports current state
rather than the stale snapshot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa0ee280-1d94-4282-a3df-2539c910e99e
📒 Files selected for processing (11)
docs/issues/yobrowser-cdp-graceful-degradation/plan.mddocs/issues/yobrowser-cdp-graceful-degradation/spec.mddocs/issues/yobrowser-cdp-graceful-degradation/tasks.mdsrc/main/presenter/browser/YoBrowserErrors.tssrc/main/presenter/browser/YoBrowserToolHandler.tssrc/main/presenter/toolPresenter/agentTools/agentToolManager.tssrc/main/presenter/toolPresenter/index.tstest/main/presenter/agentRuntimePresenter/dispatch.test.tstest/main/presenter/browser/YoBrowserToolHandler.test.tstest/main/presenter/toolPresenter/agentTools/agentToolManagerYoBrowser.test.tstest/main/presenter/toolPresenter/toolPresenter.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
yobrowser_unavailableerror contract for unavailable YoBrowser CDP callsget_browser_status/load_urlCloses #1734
Validation
pnpm run formatpnpm run i18npnpm run lintpnpm run typecheckpnpm vitest run test/main/presenter/browser/YoBrowserToolHandler.test.ts test/main/presenter/toolPresenter/agentTools/agentToolManagerYoBrowser.test.ts test/main/presenter/toolPresenter/toolPresenter.test.ts test/main/presenter/agentRuntimePresenter/dispatch.test.tsgit diff --checkSummary by CodeRabbit
New Features
Documentation