diff --git a/scripts/lint/module-boundaries-baseline.json b/scripts/lint/module-boundaries-baseline.json index d3202534bb..40524b82f5 100644 --- a/scripts/lint/module-boundaries-baseline.json +++ b/scripts/lint/module-boundaries-baseline.json @@ -73,7 +73,6 @@ "cycle-sensitive:src/platform/adapters/token/veryfront/types.ts -> #veryfront/errors", "cycle-sensitive:src/platform/adapters/veryfront-api-client/client.ts -> #veryfront/utils", "cycle-sensitive:src/platform/adapters/veryfront-api-client/operations.ts -> #veryfront/utils", - "cycle-sensitive:src/platform/adapters/veryfront-api-client/retry-handler.ts -> #veryfront/utils", "cycle-sensitive:src/platform/compat/http/websocket.ts -> #veryfront/errors", "cycle-sensitive:src/platform/compat/kv/factory.ts -> #veryfront/utils", "cycle-sensitive:src/types/entities/getEntityInfo.ts -> #veryfront/utils", diff --git a/src/platform/adapters/token/veryfront/api-client.ts b/src/platform/adapters/token/veryfront/api-client.ts index dde266e734..d992e096df 100644 --- a/src/platform/adapters/token/veryfront/api-client.ts +++ b/src/platform/adapters/token/veryfront/api-client.ts @@ -1,9 +1,11 @@ import { logger as baseLogger } from "#veryfront/utils"; -import { injectContext } from "#veryfront/observability/tracing/otlp-setup.ts"; import { type VeryfrontTokenConfig } from "./types.ts"; import { TOKEN_STORAGE_ERROR } from "#veryfront/errors/error-registry.ts"; import { VeryfrontError } from "#veryfront/errors/types.ts"; -import { retryWithBackoff } from "#veryfront/errors/error-handlers.ts"; +import { + createVeryfrontApiTransport, + type VeryfrontApiTransport, +} from "../../veryfront-api-transport.ts"; const logger = baseLogger.component("token-storage-api-client"); @@ -26,19 +28,50 @@ async function cancelResponseBody(response: Response, operation: string): Promis export class TokenStorageApiClient { private config: VeryfrontTokenConfig; + private transport: VeryfrontApiTransport; constructor(config: VeryfrontTokenConfig) { this.config = config; + + const { maxRetries, initialDelay, maxDelay } = config.retry; + const timeoutMs = config.timeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS; + + this.transport = createVeryfrontApiTransport({ + baseUrl: config.apiBaseUrl, + getToken: () => config.apiToken, + retry: { maxRetries, initialDelay, maxDelay }, + timeoutMs, + defaultHeaders: { "Accept": "application/json" }, + + onResponse: async (response) => { + // 4xx non-429: pass the response through so callers handle it. + if (response.status >= 400 && response.status < 500 && response.status !== 429) { + return response; + } + // 5xx / 429: throw to trigger retry logic. Cancel the body first so + // retries do not hold connections/buffers open. + if (!response.ok && (response.status >= 500 || response.status === 429)) { + await cancelResponseBody(response, "retry"); + throw TOKEN_STORAGE_ERROR.create({ + detail: `Server error: ${response.status}`, + status: response.status, + }); + } + return response; + }, + + wrapFinalError: (lastError) => + TOKEN_STORAGE_ERROR.create({ + detail: `Request failed after ${maxRetries} retries: ${lastError.message}`, + }), + }); } async get(key: string): Promise { const url = this.buildUrl(key); try { - const response = await this.fetchWithRetry(url, { - method: "GET", - headers: this.buildHeaders(), - }); + const response = await this.transport.request(url); if (response.status === 404) { return null; @@ -63,12 +96,9 @@ export class TokenStorageApiClient { const url = this.buildUrl(key); try { - const response = await this.fetchWithRetry(url, { + const response = await this.transport.request(url, { method: "PUT", - headers: { - ...this.buildHeaders(), - "Content-Type": "application/json", - }, + headers: { "Content-Type": "application/json" }, body: JSON.stringify({ value }), }); @@ -88,10 +118,7 @@ export class TokenStorageApiClient { const url = this.buildUrl(key); try { - const response = await this.fetchWithRetry(url, { - method: "DELETE", - headers: this.buildHeaders(), - }); + const response = await this.transport.request(url, { method: "DELETE" }); if (response.ok || response.status === 404) { return; @@ -118,10 +145,7 @@ export class TokenStorageApiClient { } try { - const response = await this.fetchWithRetry(url.toString(), { - method: "GET", - headers: this.buildHeaders(), - }); + const response = await this.transport.request(url.toString()); if (!response.ok) { await cancelResponseBody(response, "list"); @@ -167,13 +191,6 @@ export class TokenStorageApiClient { }/tokens/${encodeURIComponent(key)}`; } - private buildHeaders(): Record { - return { - Authorization: `Bearer ${this.config.apiToken}`, - Accept: "application/json", - }; - } - private wrapError( error: unknown, action: "Get" | "Set" | "Delete", @@ -190,63 +207,4 @@ export class TokenStorageApiClient { return TOKEN_STORAGE_ERROR.create({ detail: `${prefixMessage}: ${message}` }); } - - private logTimedOut(url: string, timeoutMs: number, attempt: number): void { - logger.warn("Request timed out", { - url: url.replace(/token=[^&]+/, "token=***"), - timeoutMs, - attempt: attempt + 1, - }); - } - - private async fetchWithRetry(url: string, init: RequestInit): Promise { - const { maxRetries, initialDelay, maxDelay } = this.config.retry; - const timeoutMs = this.config.timeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS; - - return retryWithBackoff( - (signal) => { - const headers = new Headers(init.headers); - injectContext(headers); - - return fetch(url, { ...init, headers, signal }).then((response) => { - if (response.status >= 400 && response.status < 500 && response.status !== 429) { - return response; - } - - if (!response.ok && (response.status >= 500 || response.status === 429)) { - throw TOKEN_STORAGE_ERROR.create({ - detail: `Server error: ${response.status}`, - status: response.status, - }); - } - - return response; - }); - }, - { - maxAttempts: maxRetries + 1, - initialDelay, - maxDelay, - timeoutMs, - onRetry: ({ error, attempt, delay, isTimeout }) => { - if (isTimeout) this.logTimedOut(url, timeoutMs, attempt); - - logger.warn("Request failed, retrying...", { - attempt: attempt + 1, - maxRetries, - delay, - error: error.message, - timeout: isTimeout, - }); - }, - wrapFinalError: (lastError, lastAttempt) => { - if (lastError.name === "AbortError") this.logTimedOut(url, timeoutMs, lastAttempt); - - return TOKEN_STORAGE_ERROR.create({ - detail: `Request failed after ${maxRetries} retries: ${lastError.message}`, - }); - }, - }, - ); - } } diff --git a/src/platform/adapters/veryfront-api-client/operations.ts b/src/platform/adapters/veryfront-api-client/operations.ts index 4cb11a2371..8c8231c7d8 100644 --- a/src/platform/adapters/veryfront-api-client/operations.ts +++ b/src/platform/adapters/veryfront-api-client/operations.ts @@ -1,5 +1,10 @@ import { logger as baseLogger } from "#veryfront/utils"; -import { type RequestOptions, requestWithRetry, type RetryConfig } from "./retry-handler.ts"; +import { + createCanonicalVeryfrontApiTransport, + type TransportRequestInit, + type TransportRetryConfig, + type VeryfrontApiTransport, +} from "../veryfront-api-transport.ts"; import { API_CLIENT_ERROR, VeryfrontError } from "./types.ts"; import { getBranchFileDetailSchema, @@ -186,16 +191,22 @@ async function listAllFiles( export class VeryfrontAPIOperations { private tokenProvider: TokenProvider; + private transport: VeryfrontApiTransport; constructor( private apiBaseUrl: string, tokenOrProvider: string | TokenProvider, - private retryConfig: RetryConfig, + retryConfig: TransportRetryConfig, private projectId?: string, ) { this.tokenProvider = typeof tokenOrProvider === "string" ? () => tokenOrProvider : tokenOrProvider; + this.transport = createCanonicalVeryfrontApiTransport( + apiBaseUrl, + () => this.tokenProvider(), + retryConfig, + ); } setTokenProvider(provider: TokenProvider): void { @@ -699,16 +710,10 @@ export class VeryfrontAPIOperations { return getReleaseAssetManifestResponseSchema().parse(raw); } - private request(endpoint: string, options: RequestOptions = {}): Promise { + private request(endpoint: string, options: TransportRequestInit = {}): Promise { return withSpan( SpanNames.API_REQUEST, - () => - requestWithRetry( - `${this.apiBaseUrl}${endpoint}`, - this.tokenProvider(), - this.retryConfig, - options, - ), + () => this.transport.request(`${this.apiBaseUrl}${endpoint}`, options), { "api.endpoint": endpoint, "api.base_url": this.apiBaseUrl }, ); } diff --git a/src/platform/adapters/veryfront-api-client/retry-handler.ts b/src/platform/adapters/veryfront-api-client/retry-handler.ts index 850b67b028..bcd428730b 100644 --- a/src/platform/adapters/veryfront-api-client/retry-handler.ts +++ b/src/platform/adapters/veryfront-api-client/retry-handler.ts @@ -1,157 +1,20 @@ -import { logger } from "#veryfront/utils"; -import { retryWithBackoff } from "#veryfront/errors/error-handlers.ts"; -import { injectContext, withSpan } from "#veryfront/observability/tracing/otlp-setup.ts"; -import { SpanNames } from "#veryfront/observability/tracing/span-names.ts"; import { - recordApiRequest, - recordApiRetry, -} from "#veryfront/observability/simple-metrics/metrics-recorder.ts"; -import { API_CLIENT_ERROR, VeryfrontError } from "./types.ts"; + createCanonicalVeryfrontApiTransport, + type TransportRequestInit, + type TransportRetryConfig, +} from "../veryfront-api-transport.ts"; -const apiLog = logger.component("api"); -const veryfrontApiClientLog = logger.component("veryfront-api-client"); +export type RetryConfig = TransportRetryConfig; +export type RequestOptions = TransportRequestInit; -export interface RetryConfig { - maxRetries: number; - initialDelay: number; - maxDelay: number; -} - -export interface RequestOptions { - returnText?: boolean; - /** Request timeout in milliseconds. Defaults to 30000ms (30 seconds). */ - timeoutMs?: number; - method?: string; - body?: BodyInit | null; - headers?: HeadersInit; - /** Demote an expected 404 miss to debug while preserving thrown error semantics. */ - expected404?: boolean; -} - -/** Default timeout for API requests (30 seconds) */ -const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; - -function logTimedOut(url: string, timeoutMs: number, attempt: number): void { - veryfrontApiClientLog.warn("Request timed out", { - url: url.replace(/token=[^&]+/, "token=***"), - timeoutMs, - attempt: attempt + 1, - }); -} - -export async function requestWithRetry( +/** Backward-compat alias; prefer holding a transport instance directly. */ +export function requestWithRetry( url: string, apiToken: string, retryConfig: RetryConfig, options: RequestOptions = {}, ): Promise { - const urlObj = new URL(url); - const urlPath = urlObj.pathname; - const { maxRetries, initialDelay, maxDelay } = retryConfig; - const timeoutMs = options.timeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS; - - // Note: We only trace the individual fetch attempts (HTTP_CLIENT_FETCH), - // not the outer retry wrapper, to reduce span nesting and trace size. - const result = await retryWithBackoff( - (signal, attempt) => { - return withSpan( - SpanNames.HTTP_CLIENT_FETCH, - async () => { - const startTime = performance.now(); - - const headers = new Headers(options.headers); - headers.set("Authorization", `Bearer ${apiToken}`); - if (!headers.has("Content-Type")) { - headers.set("Content-Type", "application/json"); - } - injectContext(headers); - - const response = await fetch(url, { - method: options.method ?? "GET", - headers, - body: options.body, - signal, - }); - const duration = performance.now() - startTime; - - recordApiRequest(response.status); - - apiLog.debug("Request completed", { - path: urlPath, - status: response.status, - durationMs: Math.round(duration), - }); - - if (!response.ok) { - const text = await response.text(); - - // Optional probes (for example stylesheet candidates) may expect a 404. - // Keep only explicit opt-ins below warn while preserving thrown error semantics. - const isExpected404 = options.expected404 === true && response.status === 404; - // 4xx = client errors (expected, e.g. 404 for missing deno.json) → warn - // 5xx = server errors (unexpected) → error - const logLevel = isExpected404 ? "debug" : response.status >= 500 ? "error" : "warn"; - veryfrontApiClientLog[logLevel]("Request failed", { - url: url.replace(/token=[^&]+/, "token=***"), - status: response.status, - statusText: response.statusText, - responseText: text.slice(0, 500), - }); - - throw API_CLIENT_ERROR.create({ - detail: `API request failed: ${response.status} ${response.statusText}`, - status: response.status, - context: { details: { url, responseText: text } }, - }); - } - - const data = options.returnText ? await response.text() : await response.json(); - return { data, status: response.status, duration }; - }, - { - "http.method": options.method ?? "GET", - "http.url": url, - "http.target": urlPath, - "http.host": urlObj.host, - "http.scheme": urlObj.protocol.replace(":", ""), - "http.retry_attempt": attempt, - }, - ); - }, - { - maxAttempts: maxRetries + 1, - initialDelay, - maxDelay, - timeoutMs, - shouldRetry: (error) => { - if (!(error instanceof VeryfrontError) || error.slug !== "api-client-error") return true; - const status = error.status; - return !status || status < 400 || status >= 500 || status === 429; - }, - onRetry: ({ error, attempt, delay, isTimeout }) => { - if (isTimeout) logTimedOut(url, timeoutMs, attempt); - - recordApiRetry(); - - veryfrontApiClientLog.warn("Request failed, retrying...", { - attempt: attempt + 1, - maxRetries, - delay, - error: error.message, - timeout: isTimeout, - }); - }, - wrapFinalError: (lastError, lastAttempt) => { - if (lastError.name === "AbortError") logTimedOut(url, timeoutMs, lastAttempt); - - return API_CLIENT_ERROR.create({ - detail: `API request failed after ${maxRetries} retries: ${lastError.message}`, - cause: lastError, - context: { details: { originalError: lastError } }, - }); - }, - }, - ); - - return result.data; + const { origin } = new URL(url); + return createCanonicalVeryfrontApiTransport(origin, () => apiToken, retryConfig) + .request(url, options); } diff --git a/src/platform/adapters/veryfront-api-transport.ts b/src/platform/adapters/veryfront-api-transport.ts new file mode 100644 index 0000000000..aefd0e6037 --- /dev/null +++ b/src/platform/adapters/veryfront-api-transport.ts @@ -0,0 +1,209 @@ +import { retryWithBackoff } from "#veryfront/errors/error-handlers.ts"; +import { API_CLIENT_ERROR } from "#veryfront/errors/error-registry.ts"; +import { VeryfrontError } from "#veryfront/errors/types.ts"; +import { injectContext, withSpan } from "#veryfront/observability/tracing/otlp-setup.ts"; +import { SpanNames } from "#veryfront/observability/tracing/span-names.ts"; +import { + recordApiRequest, + recordApiRetry, +} from "#veryfront/observability/simple-metrics/metrics-recorder.ts"; +import { serverLogger } from "#veryfront/utils/logger/logger.ts"; + +const log = serverLogger.component("veryfront-api-transport"); +const apiClientLog = serverLogger.component("veryfront-api-client"); +const DEFAULT_TIMEOUT_MS = 30_000; + +export interface TransportRetryConfig { + maxRetries: number; + initialDelay: number; + maxDelay: number; +} + +export interface TransportRequestInit { + method?: string; + headers?: HeadersInit; + body?: BodyInit | null; + returnText?: boolean; + expected404?: boolean; + timeoutMs?: number; +} + +export interface VeryfrontApiTransportConfig { + baseUrl: string; + getToken: () => string; + retry: TransportRetryConfig; + timeoutMs?: number; + defaultHeaders?: Record; + onResponse?: (response: Response, init: TransportRequestInit, url: string) => Promise; + afterFetch?: (status: number, durationMs: number) => void; + shouldRetry?: (error: unknown, attempt: number) => boolean; + onRetry?: (info: { + error: Error; + attempt: number; + delay: number; + isTimeout: boolean; + url: string; + timeoutMs: number; + }) => void; + wrapFinalError?: (lastError: Error, lastAttempt: number) => Error; + wrapFetch?: (fn: () => Promise, url: string, method: string, attempt: number) => Promise; +} + +export interface VeryfrontApiTransport { + request(pathOrUrl: string, init?: TransportRequestInit): Promise; +} + +export function createVeryfrontApiTransport( + config: VeryfrontApiTransportConfig, +): VeryfrontApiTransport { + const { + baseUrl, + getToken, + retry: { maxRetries, initialDelay, maxDelay }, + timeoutMs: cfgTimeout = DEFAULT_TIMEOUT_MS, + defaultHeaders = {}, + afterFetch, + wrapFetch, + } = config; + const onResponse = config.onResponse ?? + (defaultOnResponse as (r: Response, i: TransportRequestInit, u: string) => Promise); + const shouldRetry = config.shouldRetry ?? defaultShouldRetry; + const wrapFinalError = config.wrapFinalError ?? + ((err: Error) => + API_CLIENT_ERROR.create({ + detail: `API request failed after ${maxRetries} retries: ${err.message}`, + cause: err, + context: { details: { originalError: err } }, + })); + return { + request(pathOrUrl: string, init: TransportRequestInit = {}): Promise { + const url = pathOrUrl.startsWith("http") ? pathOrUrl : `${baseUrl}${pathOrUrl}`; + const method = init.method ?? "GET"; + const timeoutMs = init.timeoutMs ?? cfgTimeout; + // Capture the token once per request: retries of this request must not + // pick up mid-flight token mutations (setRequestToken/clearRequestToken), + // matching the pre-transport requestWithRetry semantics. + const token = getToken(); + return retryWithBackoff( + (signal, attempt) => { + const doFetch = async (): Promise => { + const headers = new Headers(init.headers); + for (const [k, v] of Object.entries(defaultHeaders)) { + if (!headers.has(k)) headers.set(k, v); + } + headers.set("Authorization", `Bearer ${token}`); + injectContext(headers); + const start = performance.now(); + const res = await fetch(url, { method, headers, body: init.body, signal }); + afterFetch?.(res.status, performance.now() - start); + return onResponse(res, init, url); + }; + return wrapFetch ? wrapFetch(doFetch, url, method, attempt) : doFetch(); + }, + { + maxAttempts: maxRetries + 1, + initialDelay, + maxDelay, + timeoutMs, + shouldRetry, + onRetry: config.onRetry + ? ({ error, attempt, delay, isTimeout }) => + config.onRetry!({ error, attempt, delay, isTimeout, url, timeoutMs }) + : ({ error, attempt, delay, isTimeout }) => { + if (isTimeout) logTimeout(url, timeoutMs, attempt); + log.warn("Request failed, retrying...", { + attempt: attempt + 1, + maxRetries, + delay, + error: error.message, + timeout: isTimeout, + }); + }, + wrapFinalError(lastError, lastAttempt) { + if (lastError.name === "AbortError") logTimeout(url, timeoutMs, lastAttempt); + return wrapFinalError(lastError, lastAttempt); + }, + }, + ); + }, + }; +} + +/** Canonical transport: span tracing, request metrics, API_CLIENT_ERROR mapping. */ +export function createCanonicalVeryfrontApiTransport( + baseUrl: string, + getToken: () => string, + retry: TransportRetryConfig, +): VeryfrontApiTransport { + return createVeryfrontApiTransport({ + baseUrl, + getToken, + retry, + defaultHeaders: { "Content-Type": "application/json" }, + afterFetch(status) { + recordApiRequest(status); + }, + onRetry({ error, attempt, delay, isTimeout, url, timeoutMs }) { + if (isTimeout) logTimeout(url, timeoutMs, attempt); + recordApiRetry(); + apiClientLog.warn("Request failed, retrying...", { + attempt: attempt + 1, + maxRetries: retry.maxRetries, + delay, + error: error.message, + timeout: isTimeout, + }); + }, + wrapFetch(fn, url, method, attempt) { + const { pathname, host, protocol } = new URL(url); + return withSpan(SpanNames.HTTP_CLIENT_FETCH, fn, { + "http.method": method, + "http.url": url, + "http.target": pathname, + "http.host": host, + "http.scheme": protocol.replace(":", ""), + "http.retry_attempt": attempt, + }); + }, + }); +} + +function logTimeout(url: string, timeoutMs: number, attempt: number): void { + log.warn("Request timed out", { + url: url.replace(/token=[^&]+/, "token=***"), + timeoutMs, + attempt: attempt + 1, + }); +} + +async function defaultOnResponse( + response: Response, + init: TransportRequestInit, + url: string, +): Promise { + if (!response.ok) { + const text = await response.text(); + const isExpected404 = init.expected404 === true && response.status === 404; + const level = isExpected404 ? "debug" : response.status >= 500 ? "error" : "warn"; + const redactedUrl = url.replace(/token=[^&]+/g, "token=***"); + apiClientLog[level]("Request failed", { + url: redactedUrl, + status: response.status, + statusText: response.statusText, + responseText: text.slice(0, 500), + }); + throw API_CLIENT_ERROR.create({ + detail: `API request failed: ${response.status} ${response.statusText}`, + status: response.status, + // Redacted so error telemetry cannot leak token query params. + context: { details: { url: redactedUrl, responseText: text } }, + }); + } + return init.returnText ? response.text() : response.json(); +} + +function defaultShouldRetry(error: unknown): boolean { + if (!(error instanceof VeryfrontError) || error.slug !== "api-client-error") return true; + const { status } = error as VeryfrontError; + return !status || status < 400 || status >= 500 || status === 429; +}