Fix race condition in async requests (#882) - #884
Merged
Conversation
Filter-driven GETs had no ordering guarantee. Clearing the inventory type filter fetches the whole repository; picking a type straight afterwards returned first, then the slow response landed and overwrote the grid, so the grid listed every card while the sidebar said "Application". - api.get/getRaw accept an optional AbortSignal, and isAbortError() lets callers tell a superseded request from a real failure. Deliberately not offered on the mutating helpers: aborting a POST does not undo it. - New shared hooks: useLatestRequest (+useAbortableEffect), useApiQuery and useDebouncedValue. They abort the predecessor and compare a generation token; the token is what actually guarantees ordering, since a resolved response cannot be aborted. - InventoryPage: cards and relations both run through useLatestRequest, so the file carries one idiom instead of two. Only the winning request clears the loading flag. Search is debounced 300ms while type and status toggles stay instant, and the grid stays in its loading state through the debounce window so it never presents stale rows as settled. A failed load now surfaces a retryable alert instead of an unhandled rejection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuDnkVPhKwF6iqBG1xRrYR
The two behavioural tests fail against a build with the generation guard disabled and pass with it, so they pin the fix rather than merely exercising it: a stale whole-repository response landing after a narrower one must not replace the rows, and a superseded request must not clear the loading state while the winner is still in flight. The AG Grid stub now exposes `loading`, mirroring its existing `select-all-rows` escape hatch, so the loading semantics are assertable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuDnkVPhKwF6iqBG1xRrYR
…#882) Matrix drew the previous axes' data underneath the newly-selected axis labels — a complete, convincing matrix with nothing on screen to say it was wrong, because `data` was never cleared and the labels render from the current selection. It now goes through useApiQuery, which discards a superseded response, and clears on switch so the existing spinner covers the gap rather than a mismatched grid. Portfolio (which also backs the Flexible Portfolio) already discarded stale responses correctly; its defect was the missing catch. Since it clears the chart on every type switch, a failed request left it on the spinner forever. It now reports the error, and aborts a superseded request instead of downloading a payload it will discard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuDnkVPhKwF6iqBG1xRrYR
Cost, Dependency, Capability Map, Lifecycle, Todos and the PPM portfolio all refetched on a picker change with nothing to order the responses, so the request for the previous selection could land last and replace the view. Each now runs through useAbortableEffect: the multi-state and Promise.all cases keep their exact shape, they just check isCurrent() before writing and pass the signal to every leg. Dependency and PPM additionally leaked their loading flag — cleared only on the success path, or by whichever request finished first. Both now clear it only when the request is still the current one. useCardSearch dropped a query change that arrived while a request was in flight and never retried it, so the picker kept showing results for a query the user had already replaced. Only a duplicate scroll-sentinel loadMore is dropped now; a new page-1 request supersedes. This engine backs CardPicker, VendorField and the diagram card sidebar, so it was reachable from nearly every picker in the app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuDnkVPhKwF6iqBG1xRrYR
Seven search-as-you-type call sites debounced with setTimeout and cleaned up with clearTimeout. That cancels the timer, but once a request has been dispatched nothing stops its response, so suggestions for a half-typed query could land after — and replace — the ones for what the user actually typed. Each now pairs useDebouncedValue with useAbortableEffect, so the pending request is aborted and a superseded response discarded. Two of them also leaked their spinner: SurveyRespond set loading outside the timer and cleared it inside, so a cancelled timer left it spinning forever. Where a spinner exists it now covers the debounce window as well as the request, so the field never looks idle while the query is still settling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuDnkVPhKwF6iqBG1xRrYR
Records the rule in CLAUDE.md so a fourth stale-guard idiom doesn't appear next to the three that had already been invented independently, including the two details that are easy to miss in review: guard every setState with isCurrent(), and clear the loading flag only when isCurrent(). UI_GUIDELINES gains the user-visible half in § 3.8 — the loading indicator covers the debounce window, a failed fetch shows an alert rather than an endless spinner, and when labels come from the current selection but the body comes from the response, a spinner beats a silent mismatch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuDnkVPhKwF6iqBG1xRrYR
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
2 participants
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
Fixes a critical race condition where slower API responses could overwrite newer ones, causing stale data to appear in the UI. Implements request serialization with abort signals and generation tokens to guarantee that only the most recent request may write state.
Type
Changes
New hooks:
useLatestRequest(): Serializes overlapping requests, aborting predecessors and using generation tokens to ensure only the newest response writes state. Handles both imperative calls (InventoryPage) and effect-based patterns.useAbortableEffect(): Effect wrapper arounduseLatestRequest()for dependency-driven refetches.useApiQuery(): Drop-in replacement foruseEffect+api.get+ state management, with built-in race condition protection and optionalselecttransform.useDebouncedValue(): Debounces rapidly-changing values (search inputs) before triggering expensive operations.API client enhancements:
GetOptionsinterface withsignal?: AbortSignalsupport.isAbortError()helper to distinguish abort rejections from real errors.api.get()andapi.getRaw()to forward abort signals to fetch.Affected pages (all now protected against [bug] Race condition caused by overlapping asynchronous filter requests #882):
useLatestRequest()for imperativeloadData()calls; debounces search input.useAbortableEffect().useApiQuery().Test coverage:
useLatestRequest(),useAbortableEffect(),useApiQuery(), anduseDebouncedValue().api.get().Test Plan
All new unit tests pass (useLatestRequest, useAbortableEffect, useApiQuery, useDebouncedValue).
InventoryPage tests verify that clearing a filter (slow whole-repository fetch) followed by picking a type doesn't let the slow response overwrite the grid.
CardSearch tests verify that changing the query mid-flight refetches instead of dropping the new query.
Existing InventoryPage, PortfolioReport, and LifecycleReport tests updated to expect abort signals in
api.get()calls.Manual verification: typing in search boxes no longer fires multiple requests; switching filters/pickers shows correct data even when responses arrive out of order.
All CI checks pass (backend lint, backend tests, frontend lint, frontend build, frontend tests)
Manually tested the affected feature
Added/updated tests for new or changed behavior
Checklist
CLAUDE.md/VERSIONand added aCHANGELOG.mdentry (user-facing fix)https://claude.ai/code/session_01KuDnkVPhKwF6iqBG1xRrYR