Repository navigation
fix(react-query): a poll that keeps answering 5xx backs off after a minute instead of every 10s - #1812
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces back-off behavior for sustained 5xx server errors in the polling mechanism, similar to the existing 400 error back-off. A one-minute grace period is applied to 5xx errors before backing off, ensuring brief outages or restarts are quickly detected. Comprehensive unit and integration tests have been added to verify this behavior, including scenarios for back-off limits, success recovery, and immediate refetching upon entity invalidation. There are no review comments to address, and the implementation looks solid.
This was referenced Oct 11, 2026
Merged
dawsontoth
added this pull request to stack #1901
October 11, 2026 21:25
dawsontoth
marked this pull request as ready for review
October 12, 2026 16:38
…inute instead of every 10s The shared poll wrapper kept every 5xx at the configured interval forever, so one tab on a cluster's instances list whose instances all answered 500 sent a get_status every 10s per instance for hours (1,262 of 1,374 5xx in one day came from one such tab). A run of 5xx now keeps the configured interval for one minute after its first settled error, then backs off exactly like a sustained 400 (the next tick waits as long as the run has lasted, capped at 5 minutes). Any success, remount, refocus, or entity invalidation still refetches at once, and 403 still stops the timer. Against the real get_status options an hour of 500s drops from 361 requests to 60. Closes #1527 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… 4.6 get_status and set_status first shipped in Harper 4.6; an older instance answers the operation with a 400. The instance page's nav bar already floored its get_status poll at 4.6.0, but the cluster instances list (one 10s poll per row) and the row menu sent it to every instance regardless of version. Both now read the instance's Harper version from the cluster record through one shared supportsGetStatus check, and a pre-4.6 row shows a muted dot explaining that availability needs Harper 4.6 or later instead of a spinner. Refs #1569 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…an unreported version Review follow-up: the row menu still derived its rotation actions from any cached availability, so it could offer set_status to an instance too old to have it; it now requires supportsGetStatus too. A row whose Harper version central manager has not reported says "Status unavailable" rather than claiming it needs an upgrade. Refs #1569 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… Harper 4.6 Review follow-up: an unrecognised version string still got the upgrade label. predatesGetStatus is true only for a version that parses and is older than the floor; anything else reads "Status unavailable". Refs #1569 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ances list Each row of a cluster's instances list polled get_status every 10s for as long as the tab was visible, so the list's cost grew with the cluster rather than with what anyone could see (one session sent 5,271 operation requests in a day). A row now polls only while it is within 200px of the viewport, through a small useIsOnScreen hook over IntersectionObserver: a visible row keeps its 10s cadence, a row scrolled out of view stops, and a row scrolling back in fetches at once if its status is stale. A row that has never loaded shows the spinner rather than an unknown dot. Browsers without IntersectionObserver poll every row as before. Refs #1603 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…erved Review follow-up: useIsOnScreen kept the last reported visibility when its element was removed or replaced, so a row that went RESTARTING while on screen, was scrolled away, and came back RUNNING would fetch once before the observer reported its new element off-screen. The state now records which element it describes and reads false for any other. Refs #1603 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hook kept the last observed element in state after the element went away, so an instances-list cell that switched to its muted branch held its detached status subtree for as long as the row stayed mounted. Reset the stale report during render, the documented pattern for adjusting state to a changed input; the return value is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… text The spinner icon is aria-hidden, so a row whose status had not loaded read as an empty cell. Off-screen rows now stay on the spinner until they scroll into view, which made that state common. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eds Harper 5.1
The deployment history list (2s/10s), the deployment detail (2s until
terminal) and the log auto-refresh (5s) were the instance-operation pollers
left outside the shared poll policy, so a 403 never stopped them and a 400 or
5xx never slowed them. The detail poll was the worst: a failed fetch has no
status to end on, so a deployment id the instance rejects ("No deployment found
with id …", a 400) was asked for every 2s for as long as the dialog stayed
open. All three now go through pollUnlessForbidden; the two whose interval
depends on the data compute it per tick and hand it over.
The deployments page is hidden from the nav below Harper 5.1, but a bookmark or
back-navigation still mounted it and polled list_deployments against an
instance that doesn't have it. It now reads the instance's version first and
says the feature needs Harper 5.1 instead of polling.
DESIGN.md records the polling convention.
Refs #1602
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on can't be read Review follow-up: if the registration_info read failed with nothing cached, the deployments page waited on the version forever behind a spinner. It now shows the existing "Could not load deployments" line instead. The log view's auto-refresh off path stays a plain `false` rather than a wrapper that always answers false. Refs #1602 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…certification A rolling deploy answers once the node that received it is done and hands the other nodes to a restart_service job (restartJobId). Studio never followed it, so a node that rejected the release read as a successful deploy. The deploy mutation now polls get_job until the job ends: a failed job rejects with DeployRolloutError naming each node, and a job Studio cannot follow resolves as unfollowable, which the callers never report as success. Also reads Harper 5.4's canary certification on a deploy's result and its rejection decision on a failure, keeps the deploy's streamed warning events, and links the progress panel to the deployment's record. Refs #1785 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n success A finished restart job with a result in neither shape Harper writes is now unknown rather than complete. The buffered redeploy and import paths report through the same rollout policy as the streamed one, so a node the legacy job could not reach, or a rollout Studio could not follow, is a warning there too, and their toasts link the deployment's record. Following the job aborts promptly when its signal fires mid-poll, and an unknown rollout settles the progress panel as inconclusive, not success. Refs #1785 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-found grace followRestartJob gave up on a job once it had been missing for 20s, but measured that from the first miss even when every poll in between failed. The receiving node restarts its own workers before its peers (harper bin/restart.ts:336-355), so a cluster address that reached another node could read one miss, then errors while the job's node restarted, and end the rollout as not-found though the job was running. A miss now only counts across an unbroken run of misses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… how each node installed The list filters by component and status, pages by the server's total, and shows each record's id, source (package, payload hash, or the release an activation made live) and user. A staged record (Harper 5.3+) is terminal and reads as waiting to be activated, so the list no longer polls it every 2s. The detail view shows the origin's install source, the activation's source release, and each peer's install comparison (5.3.1+), and drops the Restorable and Rollback-of fields no Harper version sets. The editor's Application menu links to the component's deployments. Refs #1786 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ilter where staging is absent The list no longer keeps the previous page's rows on screen, clickable, while the next page loads. A saved ?status=staged link opened against an instance older than 5.3 no longer asks for a status it cannot have. The component filter applies on Enter only, so leaving the field to click Next does not reset the page under the click, and it follows an outside change without remounting the field. Refs #1786 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the URL A saved ?status=staged link opened against Harper older than 5.3 left the URL claiming a filter while the list showed every status. Once the version is known, the page replaces the URL without it. Refs #1786 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Redeploy and Import offer Stage only where the instance reads 5.3.0 or later: it sends activate: false and no restart, and reports the staged deployment id without a health check. A staged record on the Deployments page has an Activate action, which sends deploy_component with its deployment_id and a rolling restart through the same mutation, so it is followed like any other rolling deploy; the progress panel ends a stage at Staged. All three request shapes come from one deployRequestBody, and the editor's Application menu links to a component's staged releases. Refs #1787 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sent twice Activate awaits the mutation instead of passing it callbacks, which React Query drops once the detail unmounts, so the report and the refresh still happen for an activation someone closed the dialog on. The mutation is keyed by entity and deployment, so a detail reopened mid-rollout shows it pending and offers no second activation. A stream that ends early says the outcome is unknown instead of reporting a failure. Refs #1787 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The activation reports every outcome itself, so it opts out of the global mutation toast, which would have called an early-ended stream a failure alongside the local 'may not be activated'. It logs the error with console.error as that handler did, so Datadog still sees it. Refs #1787 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…after it may have started When the SSE deploy got no event stream, onDeployComponentSubmit fell back to the buffered deploy_component on any SSEUnsupportedError: a gateway 502/503/504, a 2xx that was not a stream, a dropped connection, or the 120s idle watchdog firing before headers. Harper starts an SSE operation as soon as it accepts it and answers with a 200 stream, so each of those can follow a deploy that is already running, and the fallback deployed the same package a second time (a second install and rolling restart, or two deploys of one project racing). SSEUnsupportedError now carries the response status. The deploy falls back only on a 4xx, which Harper answers for a request it refused before running it (authentication, permissions, validation). Anything else becomes SSEInconclusiveError, which the import, redeploy and activate callers already answer by checking instead of resending. Closes #1889 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
deployComponentStream's contract still told every caller to retry on SSEUnsupportedError, which is the resend this branch removes. Also trims two comments that restated the design note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
force-pushed
the
claude/1527-poll-5xx-backoff
branch
from
October 12, 2026 16:45
e70303b to
b288492
Compare
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.
📦 This PR now carries the whole polling → deploy stack
The stacked PRs below were merged into this branch, rebased into 22 linear, signed commits on current
stage(no merge commits):get_statuson an instance older than Harper 4.6Rebase: only
DESIGN.mdindex lines needed conflict resolution. Every PR's changed lines are identical to what was reviewed in its own PR; the one cosmetic difference is #1868's index line, which now sits in the existing group without an extra blank line. Gate on the final tree:tsc -b, oxlint,dprint check, and vitest 429/429 files.Still open from #1868 (❓ your call): the deploy mutation now stays pending while it follows the rollout (up to 30 min). React Query fires
mutate()callbacks only while the caller is mounted, so leaving the applications page mid-rollout leaves the loading toast stuck and drops the outcome report. Options: report frommutateAsync().then/catchoruseMutationcallbacks, or follow the job in a separate query.⊙ Problem
pollUnlessForbidden, the sharedrefetchIntervalwrapper behind nine always-on pollers (instanceget_status, users, roles, SSH key,GET /Cluster/{id}, org/role queries, challenge certificates), stops on a 403 and backs off a sustained 400, but polled every 5xx at the configured interval forever. On 2026-09-25 one tab left on a cluster's instances list for ~2h while all 28 instances answered 500 made 1,262 of that day's 1,374 5xx on/HDBInstance/{id}/operationand/Cluster/{id}/operation, each errored tick also aconsole.errorinto Error Tracking plus a toast. RUM still shows 213 such 5xx in the last 14 days (resource events are retained ~15 days), ~85% from/instancesviews.💡 Solution
A run of 5xx now keeps the configured interval for one minute after its first settled error, then backs off exactly like the existing 400 run: the next tick waits as long as the run has outlasted the grace, clamped to [interval, 5 min]. Any success ends the run; 400s and 5xx share one run; 403 still stops the timer, and a 5xx takes the graced path; network errors, timeouts, 401/404/429 keep the base interval. Remount, refocus, and
invalidateEntityQueries(what a settled restart triggers) still refetch at once. Against the realget_statusoptions, an hour of 500s drops from 361 requests to 60.The grace clock starts at the first settled error (after the query's own retries), not the first HTTP attempt — on
get_status(retry: 3,retryDelay: 10s) that means requests keep their 10s spacing for about the first 110s, which is what keeps a restart or a brief outage visible on the next 10s tick.⚖️ Alternatives
Retry-After) — owns the cause, not the client invariant; out of this repo.🔧 Changes
src/react-query/pollUnlessForbidden.ts— the 400 run tracker becomes a failure-run tracker with a per-class grace (0 for 400, 60s for 500–599); header rewritten to the new contract.src/react-query/pollUnlessForbidden.test.ts— unit coverage for the grace, the 5-min cap, 500–599 boundaries (499/600 excluded), success reset, mixed 400/5xx runs, 5s and over-cap intervals, invalid timestamps, 403 mid-run, and other statuses unchanged.src/react-query/pollUnlessForbidden.integration.test.ts— realQueryClient/QueryObserveragainst productiongetStatusQueryOptions: an hour of 500s, the first-minute cadence, and an invalidation mid back-off followed by the 10s cadence.✅ Verification
End-to-end route: extended integration test (real React Query scheduler with production query options; network mocked). Mutation-checked: removing the 5xx branch makes 8 tests fail (the hour-long test reads 361 instead of 60); setting the grace to 0 makes 8 fail.
npx vitest run src/react-query/→ 69 passed (exit 0)npx vitest run→ 395 files / 3,676 tests passed (exit 0; also run by the pre-commit hook)npx tsc -bexit 0,npx oxlint --format stylish .exit 0,npx dprint checkexit 0Not observed against a live outage; RUM
@resource.status_code:>=500on*/operationafter deploy is the confirmation.Planning review (codex):
Framing-Verdict: chosen-approach-sound; its one major (the grace clock origin) is resolved by defining it as the first settled error, as above. Pre-push review: codex + gemini, no defects; two nits declined — studio imports are extensionless repo-wide, and the grace-clock comment is the non-obvious invariant the planning review asked to state. The cursor leg failed on the 1Password SSH agent and the domain (Harper adjudication) leg on an expired OAuth session, so outside findings were not adjudicated.Closes #1527
🤖 Generated with Claude Code
Related PRs: #1566 independent
Complexity: medium
Review-Coverage: authored=claude; ran=codex,gemini; blocked=cursor-composer(no-receipt),domain(auth); declined=cursor-grok,cursor-kimi,cursor-muse; rounds=1; full=1 @ e70303b
Review-Attention: read ~3m (raised: degraded review) @ e70303b