fix(dashboard): keep live audit inserts flowing for today-anchored ranges; silence unload stream errors - #620
Conversation
…nges; silence unload stream errors Since #617 persists the reporting window across sessions, a custom range pauses live audit inserts forever with no UI hint: restore() re-applies customStartDate/customEndDate on every load and auditLiveInsertAllowed drops every streamed event. Let a range that ends today (followsToday) keep inserts flowing — an entry arriving now is inside such a window by definition; detached historical ranges still pause inserts. Separately, every page refresh logged 'Live logs stream failed: TypeError: ...' in Firefox: unload kills the in-flight stream fetch and Firefox rejects it with plain TypeErrors ('Error in input stream', 'NetworkError when attempting to fetch resource') that isAbortError does not recognize. Abort the stream on pagehide so teardown is deliberate, trust the controller's aborted signal over the error name, and restart the stream after a back/forward-cache restore. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughLive audit logs now classify pause reasons, expose stream status, restore streams across page lifecycle events, display live status in the audit toolbar, and send an immediate heartbeat with the latest sequence. ChangesLive audit log behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant LiveLogsStore
participant AuditLogStream
participant LiveLogsHandler
participant LiveBroker
Browser->>LiveLogsStore: start live-log stream
LiveLogsStore->>LiveLogStream: consume stream
LiveLogsStore->>AuditLiveStatus: expose streaming and pause state
AuditLiveStatus-->>Browser: render Live, connecting, or paused
LiveLogsStore->>LiveLogsHandler: open subscription
LiveLogsHandler->>LiveBroker: create subscription and replay
LiveLogsHandler-->>LiveLogsStore: immediate heartbeat with latest sequence
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…tream heartbeat Add a status chip to the right of the audit list summary: a pulsing dot with 'Live' while inserts flow, and a muted dot with a how-to-resume hint while they are paused — naming the actual blocker (a date range detached from today, active filters, or a later page). The gate logic gains auditLivePauseReason() as the single source both the chip and auditLiveInsertAllowed() share. 'Group by session' moves out of the filter grid to sit next to the summary it describes. The SSE handler now writes one heartbeat immediately on subscribe: previously nothing reached the wire until the first ticker heartbeat, so response headers sat buffered for up to 15s and a freshly opened stream looked pending for the whole interval. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Added in
Verified in headless Firefox against a live build: indicator turns Live within ~2s of load, a triggered event inserts at the top of the list, a detached range shows the paused hint, and a today-anchored range stays Live. JS suite 413 pass; admin Go suite green. |
Confidence Score: 4/5Do not merge until the live-log reconnect is prevented after pagehide teardown. The date-window behavior was exercised successfully, including the safeguards for historical ranges, filters, and pagination. A production-module runtime harness reproduced a separate reconnect-after-stop lifecycle failure. Files Needing Attention: web/dashboard/src/pages/audit-logs/liveLogs.svelte.js needs a guard around the reconnect scheduled after a reader completes.
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "fix(dashboard): keep live audit inserts ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -162,7 +168,11 @@ class LiveLogsStore { | |||
| await this.consumeLiveLogsBody(res.body.getReader()); | |||
| this.scheduleLiveLogsReconnect(); | |||
There was a problem hiding this comment.
pagehide calls stopLiveLogs(), but a reader that completes normally at the same time resumes here and unconditionally schedules a reconnect. This opens a new live-log request while the page is in the back/forward cache, before pageshow performs the intended restoration. Suppress this reconnect when the controller that started the read has been stopped or replaced.
Artifacts
Production LiveLogsStore pagehide completion-race harness
- Temporary Vite-based script that runs the production module and resolves its stream reader normally immediately after pagehide stops it.
Observed pagehide completion-race output
- Command output shows the controller was cleared by pagehide and that normal stream completion scheduled one active 500 ms reconnect timer.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@internal/admin/handler_live_test.go`:
- Around line 154-171: The TestLiveLogsWritesImmediateHeartbeat regression test
currently checks the heartbeat marker and sequence ID independently, so the ID
may belong to the replayed audit event. Bind the assertion to the same SSE event
by checking the contiguous heartbeat block, such as the sequence followed by the
heartbeat event, and convert this test into a table-driven test covering the
relevant boundary cases while preserving the immediate-heartbeat behavior.
In `@internal/admin/handler_live.go`:
- Around line 61-70: The initial heartbeat in the live subscription handler must
use the sequence captured during subscription creation, not a later
liveBroker.LatestSeq() read. Update the subscription setup and Subscription type
to atomically store that sequence, then use the stored value when constructing
the heartbeat in the live handler while preserving replay event ordering.
In `@web/dashboard/src/pages/audit-logs/AuditLiveStatus.svelte`:
- Line 17: The span element with the live-dot class and is-streaming conditional
class has no corresponding CSS styles defined, so it will not render the
intended visual indicator. Add CSS style rules for the `.live-dot` class to
define its size, color, and shape properties, and add separate styles for the
`.is-streaming` modifier class to define the animation behavior that shows when
live is true. Ensure the streaming animation clearly distinguishes the live
state from the paused state.
In `@web/dashboard/src/pages/audit-logs/AuditLogsPage.svelte`:
- Around line 200-202: Remove the `.audit-log-summary` rule at lines 200-202
that sets `margin: 0`, since it is being overridden by a later
`.audit-log-summary` rule with equal specificity. Update the later
`.audit-log-summary` rule to set `margin: 0` instead of `margin-bottom: 12px` so
the intended margin alignment with the grouping control is preserved and there
is no conflicting cascade.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6e69e8c0-170d-4ec7-868c-5529cf058ce8
⛔ Files ignored due to path filters (3)
internal/admin/dashboard/static/dist/assets/index-BxPC3fFG.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-sMvqZnNv.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (8)
internal/admin/handler_live.gointernal/admin/handler_live_test.goweb/dashboard/src/pages/audit-logs/AuditFilters.svelteweb/dashboard/src/pages/audit-logs/AuditLiveStatus.svelteweb/dashboard/src/pages/audit-logs/AuditLogsPage.svelteweb/dashboard/src/pages/audit-logs/live-logs-logic.jsweb/dashboard/src/pages/audit-logs/liveLogs.svelte.jsweb/dashboard/tests/live-logs.test.js
| // An immediate heartbeat flushes the headers and gives the client its | ||
| // cursor right away; without it nothing reaches the wire until the first | ||
| // ticker heartbeat, so a freshly opened stream looked pending for up to | ||
| // the whole heartbeat interval. | ||
| if err := writeLiveEvent(res, live.Event{ | ||
| Seq: h.liveBroker.LatestSeq(), | ||
| Type: live.EventHeartbeat, | ||
| }); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline internal/live/broker.go --items all --type function,struct --match 'Subscribe|publishEvent|LatestSeq'
fi
rg -n -A80 -B15 \
'func \(b \*Broker\) Subscribe|func \(b \*Broker\) publishEvent|type Subscription' \
internal/live/broker.go
rg -n -A30 -B15 \
'Last-Event-ID|replayAfter|LatestSeq\(|Subscribe\(' \
internal/live internal/admin --glob '*.go'Repository: ENTERPILOT/GoModel
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the live handler around the reported lines and nearby cursor handling.
wc -l internal/admin/handler_live.go
sed -n '1,130p' internal/admin/handler_live.go
# Inspect Subscription fields and writeLiveEvent.
rg -n -A15 -B10 'type Subscription|func writeLiveEvent|id:|Last-Event-ID|Subscribe\(' internal/admin internal/live --glob '*.go'Repository: ENTERPILOT/GoModel
Length of output: 50374
Use the sequence captured when the subscriber is created.
Line 66 writes h.liveBroker.LatestSeq() after returning from Subscribe, while replying with replay events can already include an event that was published before this write. If publishEvent publishes sequence N to queued subscriber channels before this heartbeat writes id: N, reconnecting clients can use Last-Event-ID: N from the heartbeat and skip the queued event. Capture the latest sequence atomically during subscription setup, store it on Subscription, and use it for the initial heartbeat.
🤖 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 `@internal/admin/handler_live.go` around lines 61 - 70, The initial heartbeat
in the live subscription handler must use the sequence captured during
subscription creation, not a later liveBroker.LatestSeq() read. Update the
subscription setup and Subscription type to atomically store that sequence, then
use the stored value when constructing the heartbeat in the live handler while
preserving replay event ordering.
|
|
||
| {#if runtimeConfig.liveLogsVisible()} | ||
| <span class="audit-live-status" role="status"> | ||
| <span class="live-dot" class:is-streaming={live} aria-hidden="true"></span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define the live-dot visual states.
The component has no .live-dot or .is-streaming styles. The empty span will not render the stated live or paused indicator. Add local size, color, shape, and streaming-animation rules.
Also applies to: 28-42
🤖 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 `@web/dashboard/src/pages/audit-logs/AuditLiveStatus.svelte` at line 17, The
span element with the live-dot class and is-streaming conditional class has no
corresponding CSS styles defined, so it will not render the intended visual
indicator. Add CSS style rules for the `.live-dot` class to define its size,
color, and shape properties, and add separate styles for the `.is-streaming`
modifier class to define the animation behavior that shows when live is true.
Ensure the streaming animation clearly distinguishes the live state from the
paused state.
| .audit-log-summary { | ||
| margin: 0; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the overridden summary margin rule.
The later .audit-log-summary rule restores margin-bottom: 12px because it has equal specificity and appears after this rule. Merge the declarations or set margin: 0 in the later rule so the summary aligns with the grouping control.
🤖 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 `@web/dashboard/src/pages/audit-logs/AuditLogsPage.svelte` around lines 200 -
202, Remove the `.audit-log-summary` rule at lines 200-202 that sets `margin:
0`, since it is being overridden by a later `.audit-log-summary` rule with equal
specificity. Update the later `.audit-log-summary` rule to set `margin: 0`
instead of `margin-bottom: 12px` so the intended margin alignment with the
grouping control is preserved and there is no conflicting cascade.
…oolbar row The shared .live-dot pulse is only an expanding ring behind an 8px dot — easy to miss — so the audit chip's dot now throbs in the same rhythm, with extra clearance on the chip's left for the ring. The summary kept a pre-existing margin-bottom that broke the toolbar row's alignment; the universal reset already zeroes it, so the override simply goes away. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ter deliberate stop Heartbeats carried the broker-global LatestSeq, which can run ahead of what a connection actually received: an event published after subscribe sits queued on the subscription channel while the global counter already covers it, and the reset event had the same flaw. A client that disconnects right after such a heartbeat reconnects past the queued event and never sees it. Subscription now captures Latest atomically with the replay snapshot, and the stream tracks the newest sequence it has written (or deliberately filtered), advertising that as the reconnect cursor. Client side: a reader that ends normally right after the stream was deliberately stopped (pagehide, page teardown, restart) unconditionally scheduled a reconnect, resurrecting a stream nothing owns; the resume path now checks the read's own controller before scheduling. Review findings from CodeRabbit and Greptile on #620; the heartbeat regression test is table-driven and binds the id to the heartbeat frame. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All review findings addressed in
Suites: internal/live + internal/admin green, dashboard JS 413 pass, dist rebuilt; verified live against a running build (immediate heartbeat on connect, live insert renders, indicator states correct). |
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 (1)
web/dashboard/src/pages/audit-logs/liveLogs.svelte.js (1)
245-252: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInvalidate pending stream starts on
pagehide.
startLiveLogswaits forruntimeConfig.ensureLoaded()before it createsliveLogsController. Ifpagehideoccurs during that wait,stopLiveLogscannot abort or invalidate the pending start. When the promise resolves, it opens a stream during page teardown. Thepageshowhandler also does not restore this path becausestreamingBeforePagehidewas false.Add a lifecycle generation that
stopLiveLogsadvances. Capture it before the await and return if it changed after the await.🤖 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 `@web/dashboard/src/pages/audit-logs/liveLogs.svelte.js` around lines 245 - 252, Update startLiveLogs and stopLiveLogs to use a lifecycle generation token: capture the current generation before awaiting runtimeConfig.ensureLoaded(), and return without creating the controller if the generation changed afterward. Advance the generation inside stopLiveLogs so pagehide invalidates pending starts, while preserving the existing pageshow restoration behavior.
🤖 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 `@web/dashboard/src/pages/audit-logs/liveLogs.svelte.js`:
- Around line 245-252: Update startLiveLogs and stopLiveLogs to use a lifecycle
generation token: capture the current generation before awaiting
runtimeConfig.ensureLoaded(), and return without creating the controller if the
generation changed afterward. Advance the generation inside stopLiveLogs so
pagehide invalidates pending starts, while preserving the existing pageshow
restoration behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fd47b467-cb20-4c0c-a00b-f90c6edebd84
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-DNWYS-jJ.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (5)
internal/admin/handler_live.gointernal/admin/handler_live_test.gointernal/live/broker.gointernal/live/broker_test.goweb/dashboard/src/pages/audit-logs/liveLogs.svelte.js
Problem
Two independent issues around the live audit-log stream, found while debugging "I can't see the realtime logs":
1. Live inserts silently disabled by a persisted date range. Since #617 the reporting window persists across sessions. A custom range re-applies
customStartDate/customEndDateon every load, andauditLiveInsertAllowed()pauses live inserts whenever those are set — so one custom range selection disables live logs forever, with a connected, delivering stream and no UI hint. Verified end to end: the SSE stream was healthy (server heartbeats, events delivered in headless Chromium and Firefox), while the page dropped every event at the merge gate because localStorage held{"mode":"custom","start":"2026-07-16","end":"2026-07-31","follow":true}.2. One console error per refresh in Firefox. Page unload kills the in-flight stream fetch, and Firefox rejects it with plain TypeErrors (
Error in input streammid-read,NetworkError when attempting to fetch resourcepre-headers) instead of anAbortError, soisAbortError()misses them and every refresh logsLive logs stream failed: TypeError: …. Reproduced via Playwright Firefox:NS_BINDING_ABORTEDon the stream request at each reload.Fix
auditLiveInsertAllowed()lets a custom range that ends today (followsToday, already tracked by the date-range store) keep live inserts flowing — an entry arriving now is inside such a window by definition. Detached historical ranges still pause inserts, as before.pagehide(teardown becomes a deliberate abort), the reader trustscontroller.signal.abortedover the error's name, and a back/forward-cache restore (pageshowwithpersisted) restarts the stream if it was running.Testing
tests/live-logs.test.js(today-anchored range inserts; detached historical range still blocked); full dashboard suite: 411 pass.make frontend.🤖 Generated with Claude Code
Summary by CodeRabbit