Uh oh!
There was an error while loading. Please reload this page.
[apps][long-polling] make retries configurable & add jitter, exponential backoff and a per-attempt abort - #491
Draft
Ayc0 wants to merge 1 commit into
Draft
[apps][long-polling] make retries configurable & add jitter, exponential backoff and a per-attempt abort#491Ayc0 wants to merge 1 commit into
Ayc0 wants to merge 1 commit into
Conversation
…ial backoff and a per-attempt abort Prompts: > add the ability to backend functions to disable the long-polling feature: > https://github.com/DataDog/build-plugins/blob/a84854feb2e9af61b365f9af751683f3c0973a4e/packages/plugins/apps/src/vite/dev-server.ts#L200-L222 > > Like to say that max = 1, etc.? > Also, could we introduce 2 abilities to this? > > some jittering (so that if 3 requests are done in //, the 3 retries aren't done exactly at the same time) > some exponential backoff? > Those 2 strategies are quite standards for API auto retries > > And as the long poll is only valid for 30s, add a > const signal = new AbortSignal(); > const timeout = timeout(30); > timeout.then(() => signal.abort()); > > for (…) { > doAuthenticatedRequest(…, signal); > /code-review > create a new branch: Ayc0/retries, and /pr-ayc0-fe open it in draft
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
The dev server's long-poll loop was hardcoded to 10 attempts that fired back-to-back with no delay, and nothing bounded a single attempt — if a connection stalled it just hung. Made the retry behaviour configurable and added the two standard API auto-retry strategies (jitter + exponential backoff) on top.
Changes
apps.longPollingconfig:maxRetries(set to1to disable long-poll retrying entirely),jitter,exponentialBackoff,timeoutMs.AbortSignal.timeout(timeoutMs). A stalled attempt is abandoned and re-polled against the same receipt rather than failing the action — the receipt stays valid across attempts.timeoutMsdefaults to 40s, not 30s: the deadline needs headroom over the server's ~30s window, otherwise a healthy poll racing its own response gets aborted.d/2 + random*d/2).done: falseis the expected outcome of a healthy poll, not a failure, so the delay is there to de-synchronize concurrent pollers — every ms of it is time with no poll in flight.RequestOptsgains asignalthatdoRequestforwards tofetch(also flows through the OAuth path).Two things worth flagging for review, both caught after the first pass:
doRequestcallsbail(error)on any fetch-level rejection, so the naive version of this (just passing a signal) made things worse than no signal at all — a stall returned a 500 on attempt 1 instead of retrying. The request is now wrapped so abortscontinueand non-abort errors rethrow.instanceof Error. Node rejects with aDOMExceptionbuilt in undici's realm, which failsinstanceofacross realm boundaries (vm contexts, and the Jest env) — so the retry silently wouldn't happen. It matches onnamestructurally instead.QA Instructions
No visual change — dev-server behaviour only.
Covered by unit tests, including the paths that were broken: stall-then-retry against a real abort,
maxRetries: 1fail-fast, and non-abort errors surfacing instead of being retried away.Blast Radius
appsdev server only (/__dd/executeAction), which is alpha. Defaults keep the previous 10-attempt behaviour, so the only change for existing users is that retries are now spaced out and a hung poll recovers instead of hanging.The
signaladdition toRequestOptsis opt-in and unset everywhere else, so no other product plugin changes behaviour.