fix(replay): make a failed recordings list recoverable in place - #75849
fix(replay): make a failed recordings list recoverable in place#75849posthog[bot] wants to merge 1 commit into
Conversation
The error banner's reset key named loadSessionRecordingSuccess, which is not an action this logic has, so a successful load could never clear the error state. Fix the key, give the banner a retry button, and time the list request out so a stalled connection cannot pin the loading flag on forever. Generated-By: PostHog Code Task-Id: 8da8ec96-639f-4ad9-bf19-7f9ed6c79cb0
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.25 MiB · 22 files | no change | ███░░░░░░░ 27.7% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.11 MiB · 3,021 files | 🔺 +190 B (+0.0%) | ████████░░ 83.5% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 285.3 KiB | ../node_modules/.pnpm/posthog-js@1.409.0/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 235.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 226.9 KiB | ../node_modules/.pnpm/posthog-js@1.409.0/node_modules/posthog-js/dist/module.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.2 KiB | src/lib/api.ts |
| 94.7 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.19 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.19 MiB · 17 files | no change | ████░░░░░░ 38.2% of 5.72 MiB |
| Deferred (lazy) | 2.07 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 717.7 KiB | dist/toolbar/toolbar-app-NOGOCMN4.css |
| 546.6 KiB | dist/toolbar/chunk-chunk-45CC4NVD.js |
| 484.4 KiB | dist/toolbar/chunk-chunk-J6LEKXMR.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-INYNBXIO.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-IXFB6SXZ.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-W55RZ5CW.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-BTBOG7BH.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +7.2 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1370.56 MiB · 🔺 +7.2 KiB (+0.0%)
Problem
Two users on separate projects got permanently stuck on the session recordings list within a day of each other. One sat on an endless spinner, one on an error banner. Only a full page reload got either of them out, and neither found it.
loadSessionRecordingSuccess, singular — an action this logic does not have. The real one isloadSessionRecordingsSuccess. So a load that succeeds leaves the error state set.setFilters/setAdvancedFilters, which clear optimistically before the retry resolves, so filtering your way out just re-raises the banner on the next failure.sessionRecordingsResponseLoadingis only cleared by a success or a failure, and a stalled connection produces neither. That is the endless spinner, and it also kills "load more", which hard-returns while the flag is set.Changes
loadSessionRecordingsSuccessactually clearssessionRecordingsAPIErrored.LoadErrorBannercarrying a "Try again" action, disabled while a load is in flight.AbortControllerwith a 90s ceiling. That is far above the healthy p99, so it only fires on a request that has stalled. Because the abort surfaces as anAbortError, which the global loader handler deliberately drops, the timeout is reported explicitly — otherwise it would leave no trace anywhere.api.recordings.listtakesApiMethodOptionsso the signal can be passed through.Note
The originating report suggested the spinner came from a kea breakpoint abort. I checked kea's source and ruled that out — see "Agent context".
How did you test this code?
I'm an agent, and I did verify this visually, not only with tests.
In a browser, via Storybook. I temporarily pointed the replay home story's recordings mock at a 500, loaded the scene in Chromium and confirmed the banner renders with a working button (screenshots above). Then I flipped the mock to succeed on the retry and clicked "Try again": the list populated and the banner went away. The temporary story edit is not part of this diff.
One new test, in
Playlist.test.tsx: render the playlist against a failing list, click "Try again", and have the retry succeed with zero results. It asserts the banner is gone and the normal empty state renders. Zero results is the discriminating case — with a non-empty result the list hides the empty state and the stale error flag is invisible. Reverting the reset-key fix fails this test, restoring it passes. Nothing existing covered recovery from a failed load.sessionRecordingsPlaylistLogic.test.tschanges are mechanical: fourlistSpyassertions gainedexpect.anything()for the new second argument.Local runs
jest src/scenes/session-recordings/playlist/— 109 passed, 6 suitesjeston the other two suites that touchapi.recordings.list— 18 passedtsgo --noEmit— no new errors (287 pre-existing on master and on this branch, from unbuilt quill packages in this environment)oxlint,oxfmt,hogli ci:preflight --fix— cleanAutomatic notifications
Docs update
No user-facing docs cover this.
🤖 Agent context
Autonomy: Fully autonomous
Written by Claude (Claude Code) from an inbox report.
The report's hypothesis for the spinner was that a kea breakpoint abort leaves the loading flag set, since kea-loaders swallows breakpoints without dispatching a failure. I built that fix first, then went to kea's source to write a test for it and found the mechanism does not hold. Breakpoint counters are bumped in exactly two places: a superseded invocation of the same listener, which always dispatches a fresh load that will clear the flag; and
beforeUnmount, which only runs when the mount count reaches zero, at which point the reducers are detached and there is no state left to be stuck. So I deleted that code rather than ship a guard that can never fire. A request that never settles is the one path that does pin the flag, hence the timeout.I also considered a hard
AbortSignal.timeout(...), and dropped it: the production browserslist here isdefaults, which still includes browsers without it, and a missing global would fail every single load rather than only stalled ones.Parse hardening for the truncated-body case that the report also called out is already in
getJSONFromSuccessResponse, so there was nothing to add there.PR #75341 is open against the same loader. It moves
breakpoint()ahead of a post-await read to stop an exception on unmount. It does not touch anything here and this should merge cleanly on top of it.Skills invoked:
/writing-tests,/writing-user-facing-copy,/writing-code-comments.Created with PostHog Desktop from this inbox report.