fix(frontend): suppress stale asset PostHog errors#1883
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReworked stale-chunk detection to use a new helper module (message normalization + regex matching) for both Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Browser
participant Main as src/main.ts
participant Detection as src/services/staleAssetErrors.ts
participant PostHog as PostHog Service
Client->>Main: runtime error / unhandledrejection / preload error
Main->>Detection: getErrorMessage(error)
Detection-->>Main: normalized message
Main->>Detection: isStaleAssetErrorMessage(message)
Detection-->>Main: true (stale asset)
Main->>Main: event.preventDefault() + stopImmediatePropagation()
alt within 30s cooldown (session)
Main-->>Client: suppress reload
else outside cooldown
Main->>Main: set CHUNK_RELOAD_TIMESTAMP_KEY
Main->>Client: trigger page reload
Main->>Main: set CHUNK_RELOAD_TOAST_KEY (show post-reload toast once)
end
Note over PostHog: Asynchronous analytics filtering
Client-->>PostHog: $exception event
PostHog->>Detection: shouldSuppressPostHogExceptionEvent(event)
Detection-->>PostHog: true (suppress)
PostHog-->>PostHog: drop event (before_send returns false)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/stale-asset-errors.unit.test.ts (1)
6-47: Useit.concurrent()for these test cases.All four test cases call pure functions with no shared mutable state, making them safe candidates for parallel execution. This aligns with the repo's test parallelism pattern.
♻️ Suggested change
- it('matches the stale asset errors currently seen in PostHog', () => { + it.concurrent('matches the stale asset errors currently seen in PostHog', () => { - it('does not match unrelated runtime errors', () => { + it.concurrent('does not match unrelated runtime errors', () => { - it('extracts useful messages from arbitrary rejection values', () => { + it.concurrent('extracts useful messages from arbitrary rejection values', () => { - it('suppresses only stale asset exception events in PostHog', () => { + it.concurrent('suppresses only stale asset exception events in PostHog', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/stale-asset-errors.unit.test.ts` around lines 6 - 47, Replace the four sequential tests with concurrent tests by changing their declarations from it(...) to it.concurrent(...); specifically update the tests that call isStaleAssetErrorMessage, getErrorMessage, and shouldSuppressPostHogExceptionEvent so the specs "matches the stale asset errors...", "does not match unrelated runtime errors", "extracts useful messages from arbitrary rejection values", and "suppresses only stale asset exception events in PostHog" use it.concurrent to run in parallel (these tests call pure functions with no shared mutable state).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.ts`:
- Around line 175-179: The code clears the reload timestamp right after showing
the toast which disables the 30s cooldown; instead, stop clearing the timestamp
in this toast path and either (A) introduce a separate "toast shown" flag (e.g.,
setChunkReloadToastShown / getChunkReloadToastShown /
clearChunkReloadToastShown) to track whether the UI toast was already displayed,
or (B) only clear the reload timestamp after the cooldown window expires (e.g.,
let handleChunkError keep the timestamp for 30s and call
clearChunkReloadTimestamp once the cooldown elapses). Update the toast code to
use the new toast-flag API (or remove clearChunkReloadTimestamp here) and ensure
handleChunkError still reads getChunkReloadTimestamp to enforce the cooldown.
- Around line 88-96: The preload error handler currently suppresses all
'vite:preloadError' events and routes them to handleChunkError; change it to
first compute the message via getErrorMessage(preloadEvent.payload) ||
getErrorMessage(preloadEvent.detail) || 'Vite preload error', then call
isStaleAssetErrorMessage(message) and only if that returns true call
event.preventDefault(), event.stopImmediatePropagation(), and
handleChunkError(message). This keeps non-stale preload errors from being
suppressed while still routing genuine stale-asset messages through
handleChunkError.
In `@src/services/staleAssetErrors.ts`:
- Around line 1-9: The regex in STALE_ASSET_ERROR_PATTERNS is too broad for the
MIME-type case; tighten it to only match the HTML-fallback scenario (e.g.
messages that mention "is not a valid JavaScript MIME type" together with
"text/html" or similar) or else require the asset path context before treating
it as a stale-deploy; update the pattern in STALE_ASSET_ERROR_PATTERNS and add a
unit test that verifies non-HTML mismatches (like application/json or
text/plain) do not trigger the stale-asset detection while HTML fallback does.
---
Nitpick comments:
In `@tests/stale-asset-errors.unit.test.ts`:
- Around line 6-47: Replace the four sequential tests with concurrent tests by
changing their declarations from it(...) to it.concurrent(...); specifically
update the tests that call isStaleAssetErrorMessage, getErrorMessage, and
shouldSuppressPostHogExceptionEvent so the specs "matches the stale asset
errors...", "does not match unrelated runtime errors", "extracts useful messages
from arbitrary rejection values", and "suppresses only stale asset exception
events in PostHog" use it.concurrent to run in parallel (these tests call pure
functions with no shared mutable state).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20b8682b-9af3-47e7-aefe-274b65e94912
📒 Files selected for processing (4)
src/main.tssrc/services/posthog.tssrc/services/staleAssetErrors.tstests/stale-asset-errors.unit.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 (AI generated)
before_sendfilter so only this recoverable stale-asset class is droppedMotivation (AI generated)
The
CapgoPostHog project currently has active frontend error noise from stale asset deployments onconsole.capgo.app, including dynamic import failures, CSS preload failures, and invalid JavaScript MIME type errors. These are recoverable deployment-version mismatches rather than actionable application defects, so the app should recover automatically and avoid polluting PostHog with expected stale-asset exceptions.Business Impact (AI generated)
This reduces false-positive frontend error volume in PostHog, making real regressions easier to spot and triage. It also improves the user experience during deploy rollouts by reloading once to recover from stale assets instead of leaving users stranded on a broken chunk.
Test Plan (AI generated)
bun lintbunx vitest run tests/stale-asset-errors.unit.test.tsbun typecheckbun run buildGenerated with AI
Summary by CodeRabbit
Bug Fixes
Tests