From 4def1fd71deac2c9126eb58b2529d58c491ef069 Mon Sep 17 00:00:00 2001 From: Benjamin Koltes Date: Fri, 21 Aug 2026 22:10:24 +0200 Subject: [PATCH 1/2] [apps][long-polling] make retries configurable & add jitter, exponential backoff and a per-attempt abort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/core/src/helpers/request.ts | 3 +- packages/core/src/types.ts | 1 + packages/plugins/apps/README.md | 23 ++++ packages/plugins/apps/src/index.test.ts | 6 + packages/plugins/apps/src/types.ts | 26 ++++ packages/plugins/apps/src/validate.test.ts | 60 +++++++++ packages/plugins/apps/src/validate.ts | 23 ++++ .../plugins/apps/src/vite/dev-server.test.ts | 119 ++++++++++++++++++ packages/plugins/apps/src/vite/dev-server.ts | 101 +++++++++++++-- packages/plugins/apps/src/vite/index.test.ts | 6 + packages/plugins/apps/src/vite/index.ts | 1 + 11 files changed, 359 insertions(+), 10 deletions(-) diff --git a/packages/core/src/helpers/request.ts b/packages/core/src/helpers/request.ts index 740314ae7..a6228180a 100644 --- a/packages/core/src/helpers/request.ts +++ b/packages/core/src/helpers/request.ts @@ -90,7 +90,7 @@ export const NB_RETRIES = 5; // Do a retriable fetch. export const doRequest = async (opts: RequestOpts): Promise => { - const { auth, url, method = 'GET', getData, type = 'text' } = opts; + const { auth, url, method = 'GET', getData, type = 'text', signal } = opts; const retryOpts: retry.Options = { retries: opts.retries === 0 ? 0 : opts.retries || NB_RETRIES, onRetry: opts.onRetry, @@ -106,6 +106,7 @@ export const doRequest = async (opts: RequestOpts): Promise => { // This is needed for sending body in NodeJS' Fetch. // https://github.com/nodejs/node/issues/46221 duplex: 'half', + signal, }; let requestHeaders: RequestInit['headers'] = { 'X-Datadog-Origin': 'build-plugins', diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 164b18855..67f2f2f75 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -323,6 +323,7 @@ export type RequestOpts = { retries?: number; minTimeout?: number; maxTimeout?: number; + signal?: AbortSignal; }; export type ResolvedEntry = { name?: string; resolved: string; original: string }; diff --git a/packages/plugins/apps/README.md b/packages/plugins/apps/README.md index c9a6480bf..116e566a5 100644 --- a/packages/plugins/apps/README.md +++ b/packages/plugins/apps/README.md @@ -19,6 +19,7 @@ A plugin to upload assets to Datadog's storage - [apps.enable](#appsenable) - [apps.include](#appsinclude) - [apps.authOverrides.method](#appsauthoverridesmethod) + - [apps.longPolling](#appslongpolling) - [apps.identifier](#appsidentifier) - [apps.name](#appsname) - [apps.description](#appsdescription) @@ -46,6 +47,12 @@ apps?: { authOverrides?: { method?: 'apiKey' | 'oauth'; }; + longPolling?: { + maxRetries?: number; + jitter?: boolean; + exponentialBackoff?: boolean; + timeoutMs?: number; + }; publish?: boolean; } ``` @@ -98,6 +105,22 @@ When the method is `oauth`, the plugin derives OAuth client settings from the re For first-time authorization, the plugin starts a temporary local HTTP callback server, opens Datadog authorization in the browser, exchanges the authorization code with PKCE, and saves the returned token response for later uploads. +### apps.longPolling + +> default: `{ maxRetries: 10, jitter: true, exponentialBackoff: true, timeoutMs: 40000 }` + +Controls how the dev server's `/__dd/executeAction` endpoint polls Datadog's long-poll execution API while waiting for a backend function to finish running. + +- `maxRetries`: maximum number of long-poll attempts before giving up. Set to `1` to disable long-polling retries entirely and only poll once. +- `jitter`: randomize the delay before each retry so that several backend functions polling at the same time don't all retry in lockstep. +- `exponentialBackoff`: grow the delay between retries exponentially instead of using a fixed delay. +- `timeoutMs`: deadline for a single long-poll attempt. An attempt that stalls past it is abandoned and retried against the same receipt, so a dropped connection is re-polled instead of hanging indefinitely. + +The retry delay is capped at 2s: the server answering `done: false` is the expected outcome of a healthy poll rather than a failure, and any delay here is time with no poll in flight. + +> [!NOTE] +> `timeoutMs` must stay comfortably above the server's ~30s long-poll window. Setting it at or below that window causes healthy polls to be aborted as they race their own response. + OAuth token and authorization URLs are derived from `auth.site`, so it must match your Datadog data center (e.g. `datadoghq.com`, `us5.datadoghq.com`, `datadoghq.eu`). If `auth.site` includes a custom subdomain (e.g. `myorg.us5.datadoghq.com`), the browser is sent to that subdomain for authorization, while the token exchange and upload requests still use the base site (`us5.datadoghq.com`). ### apps.identifier diff --git a/packages/plugins/apps/src/index.test.ts b/packages/plugins/apps/src/index.test.ts index e515f20f6..a21df9385 100644 --- a/packages/plugins/apps/src/index.test.ts +++ b/packages/plugins/apps/src/index.test.ts @@ -546,6 +546,12 @@ describe('Apps Plugin - getPlugins', () => { }, dryRun: false, include: [], + longPolling: { + maxRetries: 10, + timeoutMs: 40000, + jitter: true, + exponentialBackoff: true, + }, }, }); diff --git a/packages/plugins/apps/src/types.ts b/packages/plugins/apps/src/types.ts index e7f3b18de..5bc43e471 100644 --- a/packages/plugins/apps/src/types.ts +++ b/packages/plugins/apps/src/types.ts @@ -8,6 +8,29 @@ export type AuthMethod = 'apiKey' | 'oauth'; export type AppsProtectionLevel = 'direct_publish' | 'approval_required'; +/** Controls how the dev server retries the Datadog long-poll execution endpoint. */ +export type LongPollingOptions = { + /** + * Maximum number of long-poll attempts before giving up. + * Set to `1` to disable retrying and only poll once. Default: `10`. + */ + maxRetries?: number; + /** + * Randomize the delay before each retry so that concurrent requests + * don't all retry at the exact same time. Default: `true`. + */ + jitter?: boolean; + /** Grow the delay between retries exponentially. Default: `true`. */ + exponentialBackoff?: boolean; + /** + * Deadline for a single long-poll attempt, in milliseconds. An attempt that + * stalls past it is abandoned and retried. Must stay comfortably above the + * server's ~30s long-poll window, otherwise healthy polls get aborted. + * Default: `40000`. + */ + timeoutMs?: number; +}; + export type AppsOptions = { enable?: boolean; include?: string[]; @@ -38,6 +61,8 @@ export type AppsOptions = { authOverrides?: { method?: AuthMethod; }; + /** Controls how the dev server retries the Datadog long-poll execution endpoint. */ + longPolling?: LongPollingOptions; }; export type AppsManifest = { @@ -69,6 +94,7 @@ export type AppsOptionsWithDefaults = Omit< authOverrides: { method: AuthMethod; }; + longPolling: Required; } >, 'enable' diff --git a/packages/plugins/apps/src/validate.test.ts b/packages/plugins/apps/src/validate.test.ts index 895ab4628..5b12a736d 100644 --- a/packages/plugins/apps/src/validate.test.ts +++ b/packages/plugins/apps/src/validate.test.ts @@ -27,6 +27,12 @@ describe('Apps Plugin - validateOptions', () => { include: [], identifier: undefined, name: undefined, + longPolling: { + maxRetries: 10, + timeoutMs: 40000, + jitter: true, + exponentialBackoff: true, + }, }); }); @@ -127,6 +133,12 @@ describe('Apps Plugin - validateOptions', () => { include: ['public/**/*', 'dist/**/*'], identifier: 'my-app', name: undefined, + longPolling: { + maxRetries: 10, + timeoutMs: 40000, + jitter: true, + exponentialBackoff: true, + }, }); }); @@ -181,6 +193,54 @@ describe('Apps Plugin - validateOptions', () => { expect(result.authOverrides.method).toBe('apiKey'); }); }); + + describe('longPolling', () => { + test('Should default maxRetries, jitter and exponentialBackoff', () => { + const result = validateOptions({ apps: {} }); + expect(result.longPolling).toEqual({ + maxRetries: 10, + timeoutMs: 40000, + jitter: true, + exponentialBackoff: true, + }); + }); + + test('Should allow disabling retries by setting maxRetries to 1', () => { + const result = validateOptions({ apps: { longPolling: { maxRetries: 1 } } }); + expect(result.longPolling.maxRetries).toBe(1); + }); + + test('Should allow disabling jitter and exponentialBackoff', () => { + const result = validateOptions({ + apps: { longPolling: { jitter: false, exponentialBackoff: false } }, + }); + expect(result.longPolling.jitter).toBe(false); + expect(result.longPolling.exponentialBackoff).toBe(false); + }); + + test('Should allow overriding timeoutMs', () => { + const result = validateOptions({ apps: { longPolling: { timeoutMs: 60_000 } } }); + expect(result.longPolling.timeoutMs).toBe(60_000); + }); + + test('Should throw when timeoutMs is not a positive number', () => { + expect(() => validateOptions({ apps: { longPolling: { timeoutMs: 0 } } })).toThrow( + 'apps.longPolling.timeoutMs must be a positive number.', + ); + expect(() => validateOptions({ apps: { longPolling: { timeoutMs: -1 } } })).toThrow( + 'apps.longPolling.timeoutMs must be a positive number.', + ); + }); + + test('Should throw when maxRetries is not a positive integer', () => { + expect(() => validateOptions({ apps: { longPolling: { maxRetries: 0 } } })).toThrow( + 'apps.longPolling.maxRetries must be an integer >= 1.', + ); + expect(() => validateOptions({ apps: { longPolling: { maxRetries: 1.5 } } })).toThrow( + 'apps.longPolling.maxRetries must be an integer >= 1.', + ); + }); + }); }); describe('new app properties', () => { diff --git a/packages/plugins/apps/src/validate.ts b/packages/plugins/apps/src/validate.ts index 00b8f38c3..22232fa80 100644 --- a/packages/plugins/apps/src/validate.ts +++ b/packages/plugins/apps/src/validate.ts @@ -28,6 +28,28 @@ const hasApiKeyAuth = (options: Options): boolean => (getDDEnvValue('APP_KEY') || options.auth?.appKey), ); +const resolveLongPolling = ( + longPolling: AppsOptions['longPolling'], +): AppsOptionsWithDefaults['longPolling'] => { + const maxRetries = longPolling?.maxRetries ?? 10; + const timeoutMs = longPolling?.timeoutMs ?? 40_000; + + if (!Number.isInteger(maxRetries) || maxRetries < 1) { + throw new Error('apps.longPolling.maxRetries must be an integer >= 1.'); + } + + if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) { + throw new Error('apps.longPolling.timeoutMs must be a positive number.'); + } + + return { + maxRetries, + timeoutMs, + jitter: longPolling?.jitter ?? true, + exponentialBackoff: longPolling?.exponentialBackoff ?? true, + }; +}; + export const validateOptions = (options: Options): AppsOptionsWithDefaults => { const resolvedOptions = (options[CONFIG_KEY] || {}) as AppsOptions; const method = @@ -50,5 +72,6 @@ export const validateOptions = (options: Options): AppsOptionsWithDefaults => { authOverrides: { method, }, + longPolling: resolveLongPolling(resolvedOptions.longPolling), }; }; diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index 263df3e89..fad9f790a 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -13,6 +13,7 @@ import { parseAst } from 'rollup/parseAst'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; +import type { AppsOptionsWithDefaults } from '../types'; jest.mock('@dd/core/helpers/oauth-request', () => ({ doOAuthRequest: jest.fn(async (opts) => { @@ -60,6 +61,14 @@ const mockLog = getMockLogger(); const getApiKeyRequest = () => getAuthenticatedRequest('apiKey', mockAuth, mockLog); const getOAuthRequest = () => getAuthenticatedRequest('oauth', mockOauthOnlyAuth, mockLog); +// Disable jitter/backoff so retry tests don't add unnecessary delay. +const mockLongPolling: AppsOptionsWithDefaults['longPolling'] = { + maxRetries: 10, + timeoutMs: 40000, + jitter: false, + exponentialBackoff: false, +}; + /** * Create a mock IncomingMessage with a JSON body. */ @@ -163,6 +172,7 @@ describe('Dev Server Middleware', () => { () => mockFunctions, mockAuth, getApiKeyRequest(), + mockLongPolling, '/project', mockLog, ); @@ -251,6 +261,7 @@ describe('Dev Server Middleware', () => { () => mockFunctions, mockAuth, getApiKeyRequest(), + mockLongPolling, '/project', mockLog, ); @@ -327,6 +338,7 @@ describe('Dev Server Middleware', () => { () => mockFunctions, mockAuth, getApiKeyRequest(), + mockLongPolling, '/project', mockLog, ); @@ -452,6 +464,7 @@ describe('Dev Server Middleware', () => { () => mockFunctions, mockOauthOnlyAuth, getOAuthRequest(), + mockLongPolling, '/project', mockLog, ); @@ -491,6 +504,7 @@ describe('Dev Server Middleware', () => { () => mockFunctions, mockOauthOnlyAuth, undefined, + mockLongPolling, '/project', mockLog, ); @@ -578,6 +592,7 @@ describe('Dev Server Middleware', () => { () => functionsWithAllowlist, mockAuth, getApiKeyRequest(), + mockLongPolling, '/project', mockLog, ); @@ -730,6 +745,109 @@ describe('Dev Server Middleware', () => { expect(body.result).toEqual({ data: { ok: true } }); expect(apiScope.isDone()).toBe(true); }); + + test('Should not retry when maxRetries is 1 (long-polling disabled)', async () => { + mockBuildWithParsedBackend(); + + const singleAttemptMiddleware = createDevServerMiddleware( + mockViteBuild, + () => mockFunctions, + mockAuth, + getApiKeyRequest(), + { ...mockLongPolling, maxRetries: 1 }, + '/project', + mockLog, + ); + + const apiScope = nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-no-retry' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-no-retry') + .reply(200, { data: { attributes: { done: false } } }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + singleAttemptMiddleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(500); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(false); + expect(body.error).toContain('Query execution timed out'); + expect(apiScope.isDone()).toBe(true); + }); + + test('Should retry the next attempt when a long-poll attempt stalls past timeoutMs', async () => { + mockBuildWithParsedBackend(); + + // A stalled connection must be abandoned and re-polled, not surfaced + // as a failed action: the receipt stays valid across attempts. + const stallingMiddleware = createDevServerMiddleware( + mockViteBuild, + () => mockFunctions, + mockAuth, + getApiKeyRequest(), + { ...mockLongPolling, timeoutMs: 100 }, + '/project', + mockLog, + ); + + const apiScope = nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-stall' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-stall') + .delayConnection(1_000) + .reply(200, { data: { attributes: { done: true, outputs: { data: 'late' } } } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-stall') + .reply(200, { + data: { attributes: { done: true, outputs: { data: { ok: true } } } }, + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + stallingMiddleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { ok: true } }); + expect(apiScope.isDone()).toBe(true); + }); + + test('Should surface non-abort request errors instead of retrying them away', async () => { + mockBuildWithParsedBackend(); + + const apiScope = nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-bad-request' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-bad-request') + .reply(403, { errors: [{ detail: 'Forbidden receipt' }] }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(500); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(false); + expect(body.error).toContain('Forbidden receipt'); + expect(body.error).not.toContain('Query execution timed out'); + expect(apiScope.isDone()).toBe(true); + }); }); describe('dynamic discovery', () => { @@ -740,6 +858,7 @@ describe('Dev Server Middleware', () => { () => currentFunctions, mockAuth, getApiKeyRequest(), + mockLongPolling, '/project', mockLog, ); diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index 3d0c78d58..cc7b85004 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -15,6 +15,7 @@ import { encodeQueryName } from '../backend/encodeQueryName'; import type { ExecuteActionRequest, ExecuteActionResponse } from '../backend/protocol'; import type { BackendFunction } from '../backend/types'; import { generateDevVirtualEntryContent } from '../backend/virtual-entry'; +import type { LongPollingOptions } from '../types'; import { createBackendConnectionIdCollector } from './backend-connection-id-collector'; import { getBaseBackendBuildConfig } from './build-config'; @@ -29,6 +30,57 @@ type BundleFn = (func: BackendFunction) => Promise; const DEV_VIRTUAL_PREFIX = 'virtual:dd-backend-dev:'; type AuthConfig = AuthOptionsWithDefaults; +type LongPollingConfig = Required; + +// Kept small on purpose: a `done: false` response is the expected outcome of a +// healthy poll, not a failure, and any delay here is time with no poll in +// flight. The delay exists to de-synchronize concurrent pollers, not to back +// off a broken endpoint. +const RETRY_BASE_DELAY_MS = 250; +const RETRY_MAX_DELAY_MS = 2_000; + +function delay(ms: number): Promise { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +/** + * True for the DOMException fetch rejects with when our AbortSignal fires. + * AbortSignal.timeout() aborts with a TimeoutError; an explicit abort() + * produces an AbortError. + * + * Matches structurally rather than with `instanceof Error`: the rejection is a + * DOMException built in undici's realm, which fails `instanceof` checks across + * realm boundaries (vm contexts, the Jest environment). + */ +function isAbortError(error: unknown): boolean { + if (error === null || typeof error !== 'object' || !('name' in error)) { + return false; + } + + return error.name === 'TimeoutError' || error.name === 'AbortError'; +} + +/** + * Delay before a long-poll retry attempt, combining exponential backoff and + * jitter (both standard API auto-retry strategies, and independently + * toggleable via `LongPollingConfig`). + * + * Backoff spaces out repeated retries against a slow/unhealthy endpoint. + * Jitter prevents multiple concurrent requests (e.g. several backend + * functions polling at once) from retrying in lockstep against the API. + * + * Uses equal jitter (half fixed, half random) rather than full jitter so the + * delay keeps a floor instead of collapsing towards zero. + */ +function getRetryDelay(attempt: number, config: LongPollingConfig): number { + const backoffDelay = config.exponentialBackoff + ? Math.min(RETRY_BASE_DELAY_MS * 2 ** attempt, RETRY_MAX_DELAY_MS) + : RETRY_BASE_DELAY_MS; + + return config.jitter ? backoffDelay / 2 + Math.random() * (backoffDelay / 2) : backoffDelay; +} /** Shape of the `outputs` field in a Datadog app-builder query response — * the API wraps a JS action's return value as `{ data: }`. @@ -133,6 +185,7 @@ async function executeScriptViaDatadog( args: unknown[], auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest, + longPolling: LongPollingConfig, log: Logger, ): Promise { const endpoint = `https://api.${auth.site}/api/v2/app-builder/queries/preview-async`; @@ -183,7 +236,7 @@ async function executeScriptViaDatadog( log.debug(`Query execution started with receipt: ${receiptId}`); - return pollQueryExecution(receiptId, auth, doAuthenticatedRequest, log); + return pollQueryExecution(receiptId, auth, doAuthenticatedRequest, longPolling, log); } interface PollResult { @@ -195,10 +248,11 @@ async function pollQueryExecution( receiptId: string, auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest, + longPolling: LongPollingConfig, log: Logger, ): Promise { const endpoint = `https://api.${auth.site}/api/v2/app-builder/queries/execution-long-polling/${receiptId}`; - const maxRetries = 10; + const { maxRetries, timeoutMs } = longPolling; /* * Long-poll Datadog API until the query execution completes or times out. @@ -209,17 +263,42 @@ async function pollQueryExecution( * 2. This function polls the execution-long-polling endpoint with that receipt ID. * The server holds the connection open (~30s) and responds with done: true when * the result is ready, or done: false when its long-poll window expires. + * `timeoutMs` must stay above that window so healthy polls aren't aborted. * - * This loop handles application-level re-polling (done: false), not HTTP retries. - * doRequest already retries transient HTTP failures (5xx, network errors) internally. + * This loop handles application-level re-polling (done: false) plus attempts that + * stall past LONG_POLL_TIMEOUT_MS, not HTTP retries: doRequest already retries + * transient HTTP failures (5xx, network errors) internally. + * `maxRetries: 1` effectively disables long-polling: a single request is made + * and its `done: false` response is surfaced as a timeout instead of being retried. */ for (let attempt = 0; attempt < maxRetries; attempt++) { + if (attempt > 0) { + const retryDelay = getRetryDelay(attempt, longPolling); + log.debug(`Waiting ${Math.round(retryDelay)}ms before long-poll retry...`); + await delay(retryDelay); + } + log.debug(`Long-poll attempt ${attempt + 1}/${maxRetries}...`); - const result = await doAuthenticatedRequest({ - url: endpoint, - type: 'json', - }); + let result: PollResult; + try { + result = await doAuthenticatedRequest({ + url: endpoint, + type: 'json', + // Bound the attempt so a connection that stalls past the server's + // long-poll window is abandoned rather than hanging forever. This + // covers the whole call, including doRequest's internal HTTP retries. + signal: AbortSignal.timeout(timeoutMs), + }); + } catch (error: unknown) { + // A stalled attempt is recoverable: drop this connection and poll + // again (the receipt stays valid). Anything else is a real failure. + if (!isAbortError(error)) { + throw error; + } + log.debug(`Long-poll attempt ${attempt + 1} timed out after ${timeoutMs}ms`); + continue; + } // Check for error responses. if (result.errors?.length) { @@ -237,7 +316,7 @@ async function pollQueryExecution( return attrs.outputs; } - // done === false means server-side long-poll timed out; retry immediately. + // done === false means server-side long-poll timed out; retry (subject to maxRetries). } throw new Error('Query execution timed out'); @@ -317,6 +396,7 @@ async function handleExecuteAction( bundle: BundleFn, auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest, + longPolling: LongPollingConfig, log: Logger, ): Promise { try { @@ -331,6 +411,7 @@ async function handleExecuteAction( args, auth, doAuthenticatedRequest, + longPolling, log, ); @@ -365,6 +446,7 @@ export function createDevServerMiddleware( getBackendFunctions: () => BackendFunction[], auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest | undefined, + longPolling: LongPollingConfig, projectRoot: string, log: Logger, ): (req: IncomingMessage, res: ServerResponse, next: () => void) => void { @@ -408,6 +490,7 @@ export function createDevServerMiddleware( bundle, auth, doAuthenticatedRequest, + longPolling, log, ).catch(() => { sendError(res, 500, 'Unexpected error'); diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 3a798312f..94792c6cf 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -102,6 +102,12 @@ const defaultOptions = { }, include: [], dryRun: true, + longPolling: { + maxRetries: 10, + timeoutMs: 40000, + jitter: true, + exponentialBackoff: true, + }, oauth: { authorizationUrl: 'https://api.datadoghq.com/oauth2/v1/authorize', cacheTokens: true, diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 831ce75c2..20d23bfdd 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -206,6 +206,7 @@ export const getVitePlugin = ({ getBackendFunctions, auth, doAuthenticatedRequest, + options.longPolling, context.buildRoot, log, ), From e9865bc063eced080a8dd43fd7907774f49223b4 Mon Sep 17 00:00:00 2001 From: Benjamin Koltes Date: Tue, 25 Aug 2026 19:13:14 +0200 Subject: [PATCH 2/2] [apps][long-polling] trim comments Prompts: > make your comments way less verbose Co-Authored-By: Claude Opus 5 --- packages/plugins/apps/src/types.ts | 16 ++---- packages/plugins/apps/src/vite/dev-server.ts | 57 ++++---------------- 2 files changed, 15 insertions(+), 58 deletions(-) diff --git a/packages/plugins/apps/src/types.ts b/packages/plugins/apps/src/types.ts index 5bc43e471..256a3004d 100644 --- a/packages/plugins/apps/src/types.ts +++ b/packages/plugins/apps/src/types.ts @@ -10,23 +10,15 @@ export type AppsProtectionLevel = 'direct_publish' | 'approval_required'; /** Controls how the dev server retries the Datadog long-poll execution endpoint. */ export type LongPollingOptions = { - /** - * Maximum number of long-poll attempts before giving up. - * Set to `1` to disable retrying and only poll once. Default: `10`. - */ + /** Max long-poll attempts. `1` polls once and never retries. Default: `10`. */ maxRetries?: number; - /** - * Randomize the delay before each retry so that concurrent requests - * don't all retry at the exact same time. Default: `true`. - */ + /** Randomize retry delays so concurrent pollers don't sync up. Default: `true`. */ jitter?: boolean; /** Grow the delay between retries exponentially. Default: `true`. */ exponentialBackoff?: boolean; /** - * Deadline for a single long-poll attempt, in milliseconds. An attempt that - * stalls past it is abandoned and retried. Must stay comfortably above the - * server's ~30s long-poll window, otherwise healthy polls get aborted. - * Default: `40000`. + * Deadline for one attempt, in ms. Must stay above the server's ~30s window, + * otherwise healthy polls get aborted. Default: `40000`. */ timeoutMs?: number; }; diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index cc7b85004..4505060b3 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -32,10 +32,8 @@ const DEV_VIRTUAL_PREFIX = 'virtual:dd-backend-dev:'; type AuthConfig = AuthOptionsWithDefaults; type LongPollingConfig = Required; -// Kept small on purpose: a `done: false` response is the expected outcome of a -// healthy poll, not a failure, and any delay here is time with no poll in -// flight. The delay exists to de-synchronize concurrent pollers, not to back -// off a broken endpoint. +// Kept small: `done: false` is healthy, so this delay is dead time. It only +// exists to de-synchronize concurrent pollers. const RETRY_BASE_DELAY_MS = 250; const RETRY_MAX_DELAY_MS = 2_000; @@ -45,15 +43,8 @@ function delay(ms: number): Promise { }); } -/** - * True for the DOMException fetch rejects with when our AbortSignal fires. - * AbortSignal.timeout() aborts with a TimeoutError; an explicit abort() - * produces an AbortError. - * - * Matches structurally rather than with `instanceof Error`: the rejection is a - * DOMException built in undici's realm, which fails `instanceof` checks across - * realm boundaries (vm contexts, the Jest environment). - */ +// Structural check: the rejection is a DOMException from undici's realm, so +// `instanceof` fails across realms (vm contexts, Jest). function isAbortError(error: unknown): boolean { if (error === null || typeof error !== 'object' || !('name' in error)) { return false; @@ -62,18 +53,7 @@ function isAbortError(error: unknown): boolean { return error.name === 'TimeoutError' || error.name === 'AbortError'; } -/** - * Delay before a long-poll retry attempt, combining exponential backoff and - * jitter (both standard API auto-retry strategies, and independently - * toggleable via `LongPollingConfig`). - * - * Backoff spaces out repeated retries against a slow/unhealthy endpoint. - * Jitter prevents multiple concurrent requests (e.g. several backend - * functions polling at once) from retrying in lockstep against the API. - * - * Uses equal jitter (half fixed, half random) rather than full jitter so the - * delay keeps a floor instead of collapsing towards zero. - */ +// Equal jitter (half fixed, half random) so the delay keeps a floor. function getRetryDelay(attempt: number, config: LongPollingConfig): number { const backoffDelay = config.exponentialBackoff ? Math.min(RETRY_BASE_DELAY_MS * 2 ** attempt, RETRY_MAX_DELAY_MS) @@ -255,21 +235,9 @@ async function pollQueryExecution( const { maxRetries, timeoutMs } = longPolling; /* - * Long-poll Datadog API until the query execution completes or times out. - * - * Executing an action works in two phases: - * 1. executeScriptViaDatadog sends a POST to preview-async, which starts the - * query and returns a receipt ID immediately. - * 2. This function polls the execution-long-polling endpoint with that receipt ID. - * The server holds the connection open (~30s) and responds with done: true when - * the result is ready, or done: false when its long-poll window expires. - * `timeoutMs` must stay above that window so healthy polls aren't aborted. - * - * This loop handles application-level re-polling (done: false) plus attempts that - * stall past LONG_POLL_TIMEOUT_MS, not HTTP retries: doRequest already retries - * transient HTTP failures (5xx, network errors) internally. - * `maxRetries: 1` effectively disables long-polling: a single request is made - * and its `done: false` response is surfaced as a timeout instead of being retried. + * The server holds each request open (~30s) and answers `done: false` when its + * window expires, so we re-poll. This is not an HTTP retry loop: doRequest + * already retries transient failures. `maxRetries: 1` disables re-polling. */ for (let attempt = 0; attempt < maxRetries; attempt++) { if (attempt > 0) { @@ -285,14 +253,11 @@ async function pollQueryExecution( result = await doAuthenticatedRequest({ url: endpoint, type: 'json', - // Bound the attempt so a connection that stalls past the server's - // long-poll window is abandoned rather than hanging forever. This - // covers the whole call, including doRequest's internal HTTP retries. + // Bounds the whole call, doRequest's internal retries included. signal: AbortSignal.timeout(timeoutMs), }); } catch (error: unknown) { - // A stalled attempt is recoverable: drop this connection and poll - // again (the receipt stays valid). Anything else is a real failure. + // A stall is recoverable: the receipt stays valid, so poll again. if (!isAbortError(error)) { throw error; } @@ -316,7 +281,7 @@ async function pollQueryExecution( return attrs.outputs; } - // done === false means server-side long-poll timed out; retry (subject to maxRetries). + // `done: false` means the server-side window expired; retry. } throw new Error('Query execution timed out');