feat: surface failed network requests in the console region (main) - #5
Conversation
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe renderer now reports failed XHR, fetch, and document requests as console errors. It detects HTTP and aged connection failures, deduplicates and limits output, integrates polling with snapshot and live modes, and documents the behavior with comprehensive tests. ChangesNetwork failure reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant BrowserNetwork
participant RequestLog
participant Console
Renderer->>BrowserNetwork: network()
BrowserNetwork->>RequestLog: read supported request entries
RequestLog-->>BrowserNetwork: return response entries
BrowserNetwork-->>Renderer: return normalized entries
Renderer->>Renderer: diffNetworkFailures()
Renderer->>Console: append formatted failure lines
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/plans/2026-08-03-001-feat-network-failures-console-plan.md (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the trailing space inside the code span.
markdownlint MD038 flags
`✖ `. Move the space outside the span.📝 Proposed fix
-Closes issue `#1`: failed network requests (4xx/5xx and no-response failures) appear in the pane's console region with the `✖ ` error prefix, in both poll and live-stream modes. +Closes issue `#1`: failed network requests (4xx/5xx and no-response failures) appear in the pane's console region with the `✖` error prefix, in both poll and live-stream modes.🤖 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/plans/2026-08-03-001-feat-network-failures-console-plan.md` at line 11, Update the inline code span in the plan description so it contains only the ✖ prefix, placing the trailing space outside the closing backtick to satisfy markdownlint MD038.Source: Linters/SAST tools
tests/renderer.test.mjs (1)
1848-1878: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider widening the live-timer timing budgets.
These two tests sleep 60 ms and 40 ms with a 5 ms base delay. The assertions target state transitions, not cadence, so they are correct. On a loaded CI runner a single missed firing still fails them. A poll loop that waits for the observable condition, with a longer overall deadline, removes that flake class without changing what is asserted.
🤖 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 `@tests/renderer.test.mjs` around lines 1848 - 1878, Widen the timing budgets in the live-timer tests around startNetworkTimer, replacing fixed short sleeps with polling that waits for the expected observable state transitions under a longer overall deadline. Preserve the existing assertions for network polling, painted output, and networkIdleTicks, while ensuring timers are stopped during cleanup.
🤖 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.
Inline comments:
In `@bin/renderer.mjs`:
- Around line 1473-1488: In the existing-session branch of the attach flow, keep
networkBaselinePending true before invoking pollNetwork(true), and rely on
pollNetwork to clear it after a baseline read completes. Do not clear the flag
in the not-yet-existing-session branch, where no baseline read is performed;
preserve the existing networkPollBusy guard behavior without allowing a skipped
read to mark the baseline complete.
In `@README.md`:
- Around line 206-213: Update the duplicate-collapsing description in the README
paragraph around diffNetworkFailures to state that repeated identical failures
remain collapsed while they continue recurring, with the suppression timestamp
refreshed on each suppressed hit, rather than implying a new line appears after
each fixed 60-second window.
- Around line 22-24: Update the README documentation for failed network request
reporting to state that the feature requires agent-browser 0.33 or newer, either
in that feature description or the Requirements table currently referencing
0.28.x. Keep the existing failure-reporting behavior description unchanged.
---
Nitpick comments:
In `@docs/plans/2026-08-03-001-feat-network-failures-console-plan.md`:
- Line 11: Update the inline code span in the plan description so it contains
only the ✖ prefix, placing the trailing space outside the closing backtick to
satisfy markdownlint MD038.
In `@tests/renderer.test.mjs`:
- Around line 1848-1878: Widen the timing budgets in the live-timer tests around
startNetworkTimer, replacing fixed short sleeps with polling that waits for the
expected observable state transitions under a longer overall deadline. Preserve
the existing assertions for network polling, painted output, and
networkIdleTicks, while ensuring timers are stopped during cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c437e3f-6381-4b64-8e69-83e69c2d6077
📒 Files selected for processing (4)
README.mdbin/renderer.mjsdocs/plans/2026-08-03-001-feat-network-failures-console-plan.mdtests/renderer.test.mjs
Closes #1 on
main. Same change as #3 (merged intowave0/browser-integration), rebased ontomain— the rebase was clean (wave0's renderer divergence was a header comment and asig()join character, untouched by this feature). When wave0 eventually merges to main the identical hunks resolve cleanly.Failed network requests now paint in the pane's console region —
✖ 404 GET <url>for HTTP 400–599 and✖ no response GET <url>for connection-level failures — in both polling and live-stream modes.Full design rationale, testing notes, live-smoke evidence, and deferred upstream follow-ups: see #3.
e2e: goLive…real-session test (agent-browser 0.33.2 drift, fails on unmodifiedmaintoo, skipped in CI).Post-Deploy Monitoring & Validation
No server-side deploy — ships with the plugin. After
herdr plugin link/update, close and reopen the pane, trigger a failing fetch on a localhost page, and expect a✖line within one poll interval (or ~4 s in live mode).✖ network reporting off — request log too largeon very long sessions is intended self-defense.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation