Skip to content

feat: surface failed network requests in the console region (main) - #5

Merged
Steel-tech merged 8 commits into
mainfrom
feat/network-failures-console
Aug 4, 2026
Merged

feat: surface failed network requests in the console region (main)#5
Steel-tech merged 8 commits into
mainfrom
feat/network-failures-console

Conversation

@Steel-tech

@Steel-tech Steel-tech commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Closes #1 on main. Same change as #3 (merged into wave0/browser-integration), rebased onto main — the rebase was clean (wave0's renderer divergence was a header comment and a sig() 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.

  • Tests on this base: 144/145 pass; the one failure is the pre-existing e2e: goLive… real-session test (agent-browser 0.33.2 drift, fails on unmodified main too, skipped in CI).
  • No new dependencies; Node 20 floor respected (timer code avoids Node 22+ APIs).

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 large on very long sessions is intended self-defense.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added reporting for failed XHR, fetch, and document requests.
    • Displays HTTP errors and connection failures in the console during polling and live streaming.
    • Filters duplicate reports, sanitizes URLs, and limits output volume.
    • Gracefully disables monitoring when request logs exceed execution limits.
  • Documentation

    • Documented supported failure types, detection behavior, filtering, and reporting limits.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Steel-tech, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 37617a1b-3dc6-485e-b99b-2d7819bb17c0

📥 Commits

Reviewing files that changed from the base of the PR and between 137606d and 294cade.

📒 Files selected for processing (2)
  • README.md
  • bin/renderer.mjs
📝 Walkthrough

Walkthrough

The 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.

Changes

Network failure reporting

Layer / File(s) Summary
Failure classification and browser access
bin/renderer.mjs, tests/renderer.test.mjs, docs/plans/...
The renderer reads supported request entries, detects HTTP and no-response failures, sanitizes URLs, deduplicates results, caps output, and formats console lines.
Polling and session integration
bin/renderer.mjs, tests/renderer.test.mjs, README.md
Snapshot and live polling handle baselines, backoff, attachment, navigation, cleanup, unsupported commands, and oversized logs. Tests and documentation cover the resulting behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

  • StructuPath/herdr-browser#4 — Both changes modify bin/renderer.mjs with network-failure helpers, the browser network() API, polling timers, and renderer integration.

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
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: surfacing failed network requests in the console region.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/network-failures-console

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot mentioned this pull request Aug 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 value

Remove 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 value

Consider 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

📥 Commits

Reviewing files that changed from the base of the PR and between 72b986e and 137606d.

📒 Files selected for processing (4)
  • README.md
  • bin/renderer.mjs
  • docs/plans/2026-08-03-001-feat-network-failures-console-plan.md
  • tests/renderer.test.mjs

Comment thread bin/renderer.mjs
Comment thread README.md
Comment thread README.md
@Steel-tech
Steel-tech merged commit a888739 into main Aug 4, 2026
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stream failed network requests into the console region

1 participant