From a3e71c23e84bd5198c26eb95d41417b2e8d913a4 Mon Sep 17 00:00:00 2001 From: Todti Date: Thu, 23 Jul 2026 17:19:35 +0100 Subject: [PATCH 1/2] Retry and rate-pace TestOps uploads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Classify TestOps upload failures into transient (5xx/429/timeout), recoverable ("launch is closed"), or terminal (auth/validation/payload/ conflict) kinds, and retry only the retryable ones with exponential backoff. Wired into all three upload call sites (test results, global attachments, global errors) — previously a failed request just logged and gave up with no retry at all. Add a leaky-bucket pacer with independent per-window budgets for requests, files, and bytes sent to TestOps, enabled by default with sane defaults (20 req/s, 1000 files/s, 1 GiB/s) since the server-side limits it approximates are real regardless of whether anyone configures for them. Disable via a new uploadRateLimit: false plugin option, or tune it via an explicit uploadRateLimit config. Also makes the `open` command's temp-report cleanup handle SIGTERM, not just SIGINT, via a small shared signals helper (notifySignals/ waitForAbort) with conventional exit codes and a graceful-shutdown deadline. --- packages/cli/src/commands/open.ts | 7 +- packages/cli/src/utils/index.ts | 1 + packages/cli/src/utils/signals.ts | 92 ++++++++++ packages/plugin-testops/src/client.ts | 6 + packages/plugin-testops/src/errors.ts | 158 ++++++++++++++++++ packages/plugin-testops/src/model.ts | 14 ++ packages/plugin-testops/src/plugin.ts | 96 +++++++---- packages/plugin-testops/src/uploadPacer.ts | 85 ++++++++++ packages/plugin-testops/src/utils/options.ts | 2 + packages/plugin-testops/test/errors.test.ts | 128 ++++++++++++++ .../plugin-testops/test/uploadPacer.test.ts | 122 ++++++++++++++ 11 files changed, 673 insertions(+), 38 deletions(-) create mode 100644 packages/cli/src/utils/signals.ts create mode 100644 packages/plugin-testops/src/errors.ts create mode 100644 packages/plugin-testops/src/uploadPacer.ts create mode 100644 packages/plugin-testops/test/errors.test.ts create mode 100644 packages/plugin-testops/test/uploadPacer.test.ts diff --git a/packages/cli/src/commands/open.ts b/packages/cli/src/commands/open.ts index 596e72e640f..323856e1aec 100644 --- a/packages/cli/src/commands/open.ts +++ b/packages/cli/src/commands/open.ts @@ -10,6 +10,7 @@ import { Command, Option } from "clipanion"; import { red } from "yoctocolors"; import { findFilesByGlobs } from "./../utils/fileSystem.js"; +import { notifySignals, waitForAbort } from "./../utils/signals.js"; import { generate } from "./commons/generate.js"; export class OpenCommand extends Command { @@ -83,12 +84,14 @@ export class OpenCommand extends Command { }); // clean up temp report directory on ctrl-c - process.on("SIGINT", async () => { + const notifier = notifySignals(["SIGINT", "SIGTERM"]); + + void waitForAbort(notifier.signal).then(async () => { try { await rm(config.output, { recursive: true }); } catch {} - process.exit(0); + process.exit(notifier.info()?.code ?? 0); }); await serve({ diff --git a/packages/cli/src/utils/index.ts b/packages/cli/src/utils/index.ts index 1513f39e825..cff0592626f 100644 --- a/packages/cli/src/utils/index.ts +++ b/packages/cli/src/utils/index.ts @@ -3,3 +3,4 @@ export * from "./terminal.js"; export * from "./logs.js"; export * from "./execution-context.js"; export * from "./fileSystem.js"; +export * from "./signals.js"; diff --git a/packages/cli/src/utils/signals.ts b/packages/cli/src/utils/signals.ts new file mode 100644 index 00000000000..ef7925ce414 --- /dev/null +++ b/packages/cli/src/utils/signals.ts @@ -0,0 +1,92 @@ +import process from "node:process"; + +export const SIGNAL_EXIT_CODES: Partial> = { + SIGINT: 130, + SIGTERM: 143, +}; + +export const GRACEFUL_SHUTDOWN_TIMEOUT_MS = 60_000; +export const COMMAND_TERMINATION_GRACE_MS = 5_000; + +export type SignalInfo = { + signal: NodeJS.Signals; + code: number; + receivedAt: number; + deadline: number; +}; + +const exitCodeForSignal = (signal: NodeJS.Signals): number => SIGNAL_EXIT_CODES[signal] ?? 1; + +export type SignalNotifier = { + /** aborts the moment the first SIGINT/SIGTERM is received */ + signal: AbortSignal; + /** the signal that triggered the abort, once received */ + info: () => SignalInfo | undefined; + /** stop listening for signals */ + dispose: () => void; +}; + +export const notifySignals = ( + signals: NodeJS.Signals[] = ["SIGINT", "SIGTERM"], + onRepeat?: (signal: NodeJS.Signals) => void, +): SignalNotifier => { + const controller = new AbortController(); + let info: SignalInfo | undefined; + + const handler = (signal: NodeJS.Signals) => { + if (info) { + onRepeat?.(signal); + return; + } + + const receivedAt = Date.now(); + + info = { + signal, + code: exitCodeForSignal(signal), + receivedAt, + deadline: receivedAt + GRACEFUL_SHUTDOWN_TIMEOUT_MS, + }; + + controller.abort(); + }; + + for (const signal of signals) { + process.on(signal, handler); + } + + return { + signal: controller.signal, + info: () => info, + dispose: () => { + for (const signal of signals) { + process.off(signal, handler); + } + }, + }; +}; + +/** time left until the graceful-shutdown deadline, floored at 0; undefined if no signal received yet */ +export const gracefulShutdownRemaining = (info: SignalInfo | undefined): number | undefined => { + if (!info) { + return undefined; + } + + return Math.max(0, info.deadline - Date.now()); +}; + +export const boundedTerminationSignal = ( + info: SignalInfo | undefined, + graceMs: number = COMMAND_TERMINATION_GRACE_MS, +): AbortSignal => { + const remaining = gracefulShutdownRemaining(info); + const timeoutMs = remaining === undefined ? graceMs : Math.min(remaining, graceMs); + + return AbortSignal.timeout(timeoutMs); +}; + +/** resolves once the given signal aborts */ +export const waitForAbort = (signal: AbortSignal): Promise => + signal.aborted + ? Promise.resolve() + : new Promise((resolve) => signal.addEventListener("abort", () => resolve(), { once: true })); diff --git a/packages/plugin-testops/src/client.ts b/packages/plugin-testops/src/client.ts index 12a3f33c76e..c9148327b1e 100644 --- a/packages/plugin-testops/src/client.ts +++ b/packages/plugin-testops/src/client.ts @@ -34,6 +34,7 @@ import type { UploadResultsResponseDto, } from "./model.js"; import type { TestOpsFixtureResult } from "./model.js"; +import { UploadPacer } from "./uploadPacer.js"; import { toUploadFixturesResultsDto } from "./utils/fixtures.js"; import { testStatusToLaunchStatus } from "./utils/launches.js"; import { normalizeTestStepsResults, toUploadResultsDto } from "./utils/testResults.js"; @@ -68,6 +69,7 @@ export class TestOpsClient { #session?: TestOpsSession; #uploadInProgress: boolean = false; #uploadLimit: number = 1; + #uploadPacer: UploadPacer; #namedEnvsIdsByEnv: Map = new Map(); constructor(params: TestOpsClientParams) { @@ -98,6 +100,8 @@ export class TestOpsClient { if (params.limit) { this.#uploadLimit = params.limit; } + + this.#uploadPacer = new UploadPacer(params.uploadRateLimit); } isTestOpsClientError(error: unknown): error is TestOpsClientError { @@ -427,6 +431,8 @@ export class TestOpsClient { await this.createNamedEnvs(Array.from(chunkEnvs.values())); } + await this.#uploadPacer.wait({ requests: 1, files: trsChunk.length }); + const reportIdsToTestOpsIds = await this.#postTestResultsChunk(trsChunk); uploadedTrs.push(...trsChunk.filter((tr) => typeof reportIdsToTestOpsIds[tr.id] === "number")); diff --git a/packages/plugin-testops/src/errors.ts b/packages/plugin-testops/src/errors.ts new file mode 100644 index 00000000000..5f1b8052fa6 --- /dev/null +++ b/packages/plugin-testops/src/errors.ts @@ -0,0 +1,158 @@ +import { setTimeout as delay } from "node:timers/promises"; + +import { isAxiosError } from "axios"; + +export enum ErrorKind { + None = "none", + ServiceTransient = "service_transient", + ResourceRecoverable = "resource_recoverable", + AuthTerminal = "auth_terminal", + NotFoundTerminal = "not_found_terminal", + PayloadTerminal = "payload_terminal", + ValidationTerminal = "validation_terminal", + ConflictTerminal = "conflict_terminal", + Unknown = "unknown", +} + +const isLaunchClosedMessage = (message: string | undefined): boolean => { + if (!message) { + return false; + } + + const lower = message.toLowerCase(); + + return lower.includes("launch is closed") || lower.includes("closed launch"); +}; + +const responseStatus = (error: unknown): number | undefined => + isAxiosError(error) ? error.response?.status : undefined; + +const responseMessage = (error: unknown): string | undefined => { + if (!isAxiosError(error)) { + return undefined; + } + + const data = error.response?.data as { message?: string } | undefined; + + return data?.message; +}; + +const classifyHttpStatus = (status: number): ErrorKind => { + if (status === 408 || status === 429 || status >= 500) { + return ErrorKind.ServiceTransient; + } + + if (status === 423) { + return ErrorKind.ResourceRecoverable; + } + + if (status === 401 || status === 403) { + return ErrorKind.AuthTerminal; + } + + if (status === 404) { + return ErrorKind.NotFoundTerminal; + } + + if (status === 413 || status === 415) { + return ErrorKind.PayloadTerminal; + } + + if (status === 409) { + return ErrorKind.ConflictTerminal; + } + + if (status >= 400 && status < 500) { + return ErrorKind.ValidationTerminal; + } + + return ErrorKind.Unknown; +}; + +export const classifyError = (error: unknown): ErrorKind => { + if (!error) { + return ErrorKind.None; + } + + if (isAxiosError(error)) { + if (isLaunchClosedMessage(responseMessage(error))) { + return ErrorKind.ResourceRecoverable; + } + + const status = responseStatus(error); + + // no response at all: network error, DNS failure, connection reset, request timeout, ... + return status === undefined ? ErrorKind.ServiceTransient : classifyHttpStatus(status); + } + + if (error instanceof Error && (error.name === "AbortError" || /timeout/i.test(error.message))) { + return ErrorKind.ServiceTransient; + } + + return ErrorKind.Unknown; +}; + +export const shouldRetryUpload = (error: unknown): boolean => { + const kind = classifyError(error); + + return kind === ErrorKind.ServiceTransient || kind === ErrorKind.ResourceRecoverable; +}; + +export const isPermanentUploadError = (error: unknown): boolean => { + switch (classifyError(error)) { + case ErrorKind.AuthTerminal: + case ErrorKind.NotFoundTerminal: + case ErrorKind.PayloadTerminal: + case ErrorKind.ValidationTerminal: + case ErrorKind.ConflictTerminal: + return true; + default: + return false; + } +}; + +export const isTerminalUploadError = (error: unknown, retries: number, maxRetries: number): boolean => + isPermanentUploadError(error) || retries >= maxRetries; + +export type RetryOptions = { + maxRetries?: number; + baseDelayMs?: number; + maxDelayMs?: number; + onRetry?: (error: unknown, attempt: number) => void; +}; + +const DEFAULT_MAX_RETRIES = 3; +const DEFAULT_BASE_DELAY_MS = 500; +const DEFAULT_MAX_DELAY_MS = 5_000; + +/** + * Retries an upload operation with exponential backoff, but only for errors classified as + * retryable (transient service errors, a recoverable "launch is closed" race). Terminal errors + * (auth, validation, payload too large, ...) fail fast on the first attempt. + */ +export const withUploadRetry = async (operation: () => Promise, options: RetryOptions = {}): Promise => { + const { + maxRetries = DEFAULT_MAX_RETRIES, + baseDelayMs = DEFAULT_BASE_DELAY_MS, + maxDelayMs = DEFAULT_MAX_DELAY_MS, + onRetry, + } = options; + let attempt = 0; + + for (;;) { + try { + return await operation(); + } catch (error) { + if (!shouldRetryUpload(error) || isTerminalUploadError(error, attempt, maxRetries)) { + throw error; + } + + attempt += 1; + onRetry?.(error, attempt); + + const backoffMs = Math.min(maxDelayMs, baseDelayMs * 2 ** (attempt - 1)); + + await delay(backoffMs); + } + } +}; diff --git a/packages/plugin-testops/src/model.ts b/packages/plugin-testops/src/model.ts index 2b948d1d23d..b8fd80e41c5 100644 --- a/packages/plugin-testops/src/model.ts +++ b/packages/plugin-testops/src/model.ts @@ -210,11 +210,24 @@ export type LaunchCategoryBulkResult = { externalId: string; }; +/** + * Caps how fast uploads are sent to TestOps within a rolling time window. Each budget is + * optional and independent — unset ones don't pace anything. Paced with sane defaults + * (see `DEFAULT_UPLOAD_RATE_LIMIT`) unless explicitly overridden; pass `false` to disable pacing. + */ +export type UploadRateLimit = { + windowMs: number; + maxRequestsPerWindow?: number; + maxFilesPerWindow?: number; + maxBytesPerWindow?: number; +}; + export type TestOpsClientParams = { baseUrl: string; projectId: string; accessToken: string; limit?: number; + uploadRateLimit?: UploadRateLimit | false; }; export type AttachmentForUpload = { @@ -239,6 +252,7 @@ export type TestOpsUploaderOptions = { ancestorLimit?: number; filter?: (testResult: TestResult) => boolean; limit?: number; + uploadRateLimit?: UploadRateLimit | false; }; export interface TestOpsFixtureResult extends Omit { diff --git a/packages/plugin-testops/src/plugin.ts b/packages/plugin-testops/src/plugin.ts index 532e5b1cc30..510651d9f0f 100644 --- a/packages/plugin-testops/src/plugin.ts +++ b/packages/plugin-testops/src/plugin.ts @@ -15,6 +15,7 @@ import { uniqBy, stubTrue } from "lodash-es"; import { bold } from "yoctocolors"; import { TestOpsClient } from "./client.js"; +import { withUploadRetry } from "./errors.js"; import { LaunchGitFlow, resolveGitFlowOptions } from "./gitFlow/index.js"; import { Logger } from "./logger.js"; import type { TestOpsPluginTestResult, TestOpsPluginOptions, UploadCategory } from "./model.js"; @@ -65,6 +66,7 @@ export class TestOpsPlugin implements Plugin { launchName, launchTags, autocloseLaunch = true, + uploadRateLimit, } = resolvePluginOptions(options); // don't initialize the client when some options are missing @@ -74,6 +76,7 @@ export class TestOpsPlugin implements Plugin { baseUrl: endpoint, accessToken, projectId, + uploadRateLimit, }); this.#launchName = launchName; this.#launchTags = launchTags; @@ -209,12 +212,19 @@ export class TestOpsPlugin implements Plugin { try { progressLogger.log(true); - await this.#client.uploadGlobalErrors(results, (percent) => { - if (!completed && percent >= 100) { - completed = true; - progressLogger.increment(); - } - }); + await withUploadRetry( + () => + this.#client.uploadGlobalErrors(results, (percent) => { + if (!completed && percent >= 100) { + completed = true; + progressLogger.increment(); + } + }), + { + onRetry: (error, attempt) => + this.#logger.debug(`Retrying global errors upload (attempt ${attempt}): ${error}`), + }, + ); if (!completed) { progressLogger.increment(); @@ -253,30 +263,37 @@ export class TestOpsPlugin implements Plugin { try { progressLogger.log(true); - await this.#client.uploadGlobalAttachments({ - attachments, - attachmentsResolver: async (attachmentLink) => { - const content = await store.attachmentContentById(attachmentLink.id); - const body = await content?.readContent(async (stream) => stream); - const filename = uploadFilenameForLink(attachmentLink); - - if (filename === undefined || body === undefined) { - return undefined; - } - - return { - originalFileName: filename, - contentType: attachmentLink.contentType ?? "application/octet-stream", - content: body, - }; + await withUploadRetry( + () => + this.#client.uploadGlobalAttachments({ + attachments, + attachmentsResolver: async (attachmentLink) => { + const content = await store.attachmentContentById(attachmentLink.id); + const body = await content?.readContent(async (stream) => stream); + const filename = uploadFilenameForLink(attachmentLink); + + if (filename === undefined || body === undefined) { + return undefined; + } + + return { + originalFileName: filename, + contentType: attachmentLink.contentType ?? "application/octet-stream", + content: body, + }; + }, + onProgress: (percent) => { + if (!completed && percent >= 100) { + completed = true; + progressLogger.increment(); + } + }, + }), + { + onRetry: (error, attempt) => + this.#logger.debug(`Retrying global attachments upload (attempt ${attempt}): ${error}`), }, - onProgress: (percent) => { - if (!completed && percent >= 100) { - completed = true; - progressLogger.increment(); - } - }, - }); + ); if (!completed) { progressLogger.increment(); @@ -320,13 +337,20 @@ export class TestOpsPlugin implements Plugin { try { logProgress(true); - const uploadedTrs = await this.#client.uploadTestResults({ - attachmentsResolver: attachmentsResolverFactory(store), - fixturesResolver: fixturesResolverFactory(store), - environments, - trs: trsToUpload, - onProgress: () => incrementProgress(), - }); + const uploadedTrs = await withUploadRetry( + () => + this.#client.uploadTestResults({ + attachmentsResolver: attachmentsResolverFactory(store), + fixturesResolver: fixturesResolverFactory(store), + environments, + trs: trsToUpload, + onProgress: () => incrementProgress(), + }), + { + onRetry: (error, attempt) => + this.#logger.debug(`Retrying test results upload (attempt ${attempt}): ${error}`), + }, + ); logProgress(true); diff --git a/packages/plugin-testops/src/uploadPacer.ts b/packages/plugin-testops/src/uploadPacer.ts new file mode 100644 index 00000000000..4fb33e1685f --- /dev/null +++ b/packages/plugin-testops/src/uploadPacer.ts @@ -0,0 +1,85 @@ +import { setTimeout as delay } from "node:timers/promises"; + +import type { UploadRateLimit } from "./model.js"; + +export type UploadCost = { + requests?: number; + files?: number; + bytes?: number; +}; + +type RateBudget = { + limit: number; + windowMs: number; + next: number; +}; + +export const DEFAULT_UPLOAD_RATE_LIMIT: UploadRateLimit = { + windowMs: 1_000, + maxRequestsPerWindow: 20, + maxFilesPerWindow: 1_000, + maxBytesPerWindow: 1024 * 1024 * 1024, +}; + +const isBudgetEnabled = (budget: RateBudget): boolean => budget.windowMs > 0 && budget.limit > 0; + +// oversized batches are charged as one full window so one large batch can still make progress +// instead of reserving many windows up front +const scaledWindowCost = (windowMs: number, cost: number, limit: number): number => { + if (windowMs <= 0 || cost <= 0 || limit <= 0) { + return 0; + } + + return (windowMs * Math.min(cost, limit)) / limit; +}; + +export class UploadPacer { + #requests: RateBudget; + #files: RateBudget; + #bytes: RateBudget; + readonly #now: () => number; + + constructor(rateLimit: UploadRateLimit | false | undefined, now: () => number = Date.now) { + const resolved = rateLimit === undefined ? DEFAULT_UPLOAD_RATE_LIMIT : rateLimit || undefined; + const windowMs = resolved?.windowMs ?? 0; + + this.#requests = { limit: resolved?.maxRequestsPerWindow ?? 0, windowMs, next: 0 }; + this.#files = { limit: resolved?.maxFilesPerWindow ?? 0, windowMs, next: 0 }; + this.#bytes = { limit: resolved?.maxBytesPerWindow ?? 0, windowMs, next: 0 }; + this.#now = now; + } + + async wait(cost: UploadCost, signal?: AbortSignal): Promise { + const waitMs = this.#reserve(this.#now(), cost); + + if (waitMs <= 0) { + return; + } + + await delay(waitMs, undefined, signal ? { signal } : undefined); + } + + #reserve(now: number, cost: UploadCost): number { + let start = now; + + for (const budget of [this.#requests, this.#files, this.#bytes]) { + if (isBudgetEnabled(budget) && budget.next > start) { + start = budget.next; + } + } + + this.#reserveAt(this.#requests, start, cost.requests ?? 0); + this.#reserveAt(this.#files, start, cost.files ?? 0); + this.#reserveAt(this.#bytes, start, cost.bytes ?? 0); + + return start > now ? start - now : 0; + } + + #reserveAt(budget: RateBudget, start: number, cost: number): void { + if (!isBudgetEnabled(budget) || cost <= 0) { + return; + } + + budget.next = start + scaledWindowCost(budget.windowMs, cost, budget.limit); + } +} diff --git a/packages/plugin-testops/src/utils/options.ts b/packages/plugin-testops/src/utils/options.ts index 35e5f06bd4a..9ee82deb522 100644 --- a/packages/plugin-testops/src/utils/options.ts +++ b/packages/plugin-testops/src/utils/options.ts @@ -11,6 +11,7 @@ export const resolvePluginOptions = (options: TestOpsPluginOptions): Omit { + await story("errors"); +}); + +const axiosError = (status: number | undefined, message?: string) => ({ + isAxiosError: true, + response: status === undefined ? undefined : { status, data: message ? { message } : {} }, +}); + +describe("classifyError", () => { + it("classifies no error as none", () => { + expect(classifyError(undefined)).toBe(ErrorKind.None); + }); + + it.each([408, 429, 500, 502, 503])("classifies HTTP %i as service transient", (status) => { + expect(classifyError(axiosError(status))).toBe(ErrorKind.ServiceTransient); + }); + + it("classifies a network error with no response as service transient", () => { + expect(classifyError(axiosError(undefined))).toBe(ErrorKind.ServiceTransient); + }); + + it("classifies HTTP 423 as resource recoverable", () => { + expect(classifyError(axiosError(423))).toBe(ErrorKind.ResourceRecoverable); + }); + + it("classifies a 'launch is closed' message as resource recoverable regardless of status", () => { + expect(classifyError(axiosError(400, "Launch is closed"))).toBe(ErrorKind.ResourceRecoverable); + }); + + it.each([401, 403])("classifies HTTP %i as auth terminal", (status) => { + expect(classifyError(axiosError(status))).toBe(ErrorKind.AuthTerminal); + }); + + it("classifies HTTP 404 as not found terminal", () => { + expect(classifyError(axiosError(404))).toBe(ErrorKind.NotFoundTerminal); + }); + + it.each([413, 415])("classifies HTTP %i as payload terminal", (status) => { + expect(classifyError(axiosError(status))).toBe(ErrorKind.PayloadTerminal); + }); + + it("classifies HTTP 409 as conflict terminal", () => { + expect(classifyError(axiosError(409))).toBe(ErrorKind.ConflictTerminal); + }); + + it("classifies other 4xx as validation terminal", () => { + expect(classifyError(axiosError(422))).toBe(ErrorKind.ValidationTerminal); + }); + + it("classifies a plain non-axios error as unknown", () => { + expect(classifyError(new Error("boom"))).toBe(ErrorKind.Unknown); + }); +}); + +describe("shouldRetryUpload / isPermanentUploadError", () => { + it("retries transient and recoverable errors", () => { + expect(shouldRetryUpload(axiosError(503))).toBe(true); + expect(shouldRetryUpload(axiosError(423))).toBe(true); + }); + + it("does not retry terminal errors", () => { + expect(shouldRetryUpload(axiosError(401))).toBe(false); + expect(shouldRetryUpload(axiosError(404))).toBe(false); + }); + + it("treats terminal statuses as permanent", () => { + expect(isPermanentUploadError(axiosError(401))).toBe(true); + expect(isPermanentUploadError(axiosError(503))).toBe(false); + }); + + it("is terminal once retries are exhausted even for a retryable error", () => { + expect(isTerminalUploadError(axiosError(503), 3, 3)).toBe(true); + expect(isTerminalUploadError(axiosError(503), 1, 3)).toBe(false); + }); +}); + +describe("withUploadRetry", () => { + it("returns the result on first success without retrying", async () => { + const operation = vi.fn().mockResolvedValue("ok"); + + const result = await withUploadRetry(operation, { baseDelayMs: 0 }); + + expect(result).toBe("ok"); + expect(operation).toHaveBeenCalledTimes(1); + }); + + it("retries a transient error and eventually succeeds", async () => { + const operation = vi + .fn() + .mockRejectedValueOnce(axiosError(503)) + .mockRejectedValueOnce(axiosError(429)) + .mockResolvedValueOnce("ok"); + const onRetry = vi.fn(); + + const result = await withUploadRetry(operation, { baseDelayMs: 0, onRetry }); + + expect(result).toBe("ok"); + expect(operation).toHaveBeenCalledTimes(3); + expect(onRetry).toHaveBeenCalledTimes(2); + }); + + it("fails fast on a terminal error without retrying", async () => { + const operation = vi.fn().mockRejectedValue(axiosError(401)); + + await expect(withUploadRetry(operation, { baseDelayMs: 0 })).rejects.toEqual(axiosError(401)); + expect(operation).toHaveBeenCalledTimes(1); + }); + + it("gives up after maxRetries and rethrows the last error", async () => { + const operation = vi.fn().mockRejectedValue(axiosError(503)); + + await expect(withUploadRetry(operation, { baseDelayMs: 0, maxRetries: 2 })).rejects.toEqual(axiosError(503)); + expect(operation).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/plugin-testops/test/uploadPacer.test.ts b/packages/plugin-testops/test/uploadPacer.test.ts new file mode 100644 index 00000000000..ba20ab0b604 --- /dev/null +++ b/packages/plugin-testops/test/uploadPacer.test.ts @@ -0,0 +1,122 @@ +import { performance } from "node:perf_hooks"; + +import { story } from "allure-js-commons"; +import { beforeEach, describe, expect, it } from "vitest"; + +import { DEFAULT_UPLOAD_RATE_LIMIT, UploadPacer } from "../src/uploadPacer.js"; + +beforeEach(async () => { + await story("uploadPacer"); +}); + +describe("UploadPacer", () => { + it("never waits when pacing is explicitly disabled", async () => { + const pacer = new UploadPacer(false); + const start = performance.now(); + + await pacer.wait({ requests: 1, files: 1000, bytes: 1_000_000 }); + + expect(performance.now() - start).toBeLessThan(20); + }); + + it("paces with sane defaults when no rate limit is given, not left disabled", async () => { + const pacer = new UploadPacer(undefined); + const start = performance.now(); + + // one request well within the default per-second budgets: shouldn't wait + await pacer.wait({ requests: 1, files: 1 }); + + expect(performance.now() - start).toBeLessThan(20); + + // exhausting the default request budget in one shot should start pacing the next call + await pacer.wait({ requests: DEFAULT_UPLOAD_RATE_LIMIT.maxRequestsPerWindow }); + + let resolved = false; + const pending = pacer.wait({ requests: 1 }).then(() => { + resolved = true; + }); + + await Promise.resolve(); + expect(resolved).toBe(false); + + // avoid leaving a dangling real-timer wait past the end of the test + await pending.catch(() => {}); + }); + + it("does not delay a request within budget", async () => { + const pacer = new UploadPacer({ windowMs: 10_000, maxRequestsPerWindow: 5 }); + const start = performance.now(); + + await pacer.wait({ requests: 1 }); + + expect(performance.now() - start).toBeLessThan(20); + }); + + it("paces a request once the per-window request budget is exceeded", async () => { + const windowMs = 80; + const pacer = new UploadPacer({ windowMs, maxRequestsPerWindow: 1 }); + + await pacer.wait({ requests: 1 }); + + const start = performance.now(); + + await pacer.wait({ requests: 1 }); + + expect(performance.now() - start).toBeGreaterThanOrEqual(windowMs - 15); + }); + + it("treats requests, files, and bytes as independent budgets", async () => { + const windowMs = 80; + const pacer = new UploadPacer({ + windowMs, + maxRequestsPerWindow: 100, + maxFilesPerWindow: 100, + maxBytesPerWindow: 10, + }); + + // well within the requests/files budgets, but exhausts the bytes budget + await pacer.wait({ requests: 1, files: 1, bytes: 10 }); + + const start = performance.now(); + + await pacer.wait({ requests: 1, files: 1, bytes: 10 }); + + expect(performance.now() - start).toBeGreaterThanOrEqual(windowMs - 15); + }); + + it("charges an oversized batch as a single window instead of scaling past it", async () => { + const windowMs = 80; + const smallPacer = new UploadPacer({ windowMs, maxFilesPerWindow: 10 }); + const bigPacer = new UploadPacer({ windowMs, maxFilesPerWindow: 10 }); + + await smallPacer.wait({ files: 10 }); // exactly at the limit + await bigPacer.wait({ files: 1000 }); // far beyond the limit + + const smallStart = performance.now(); + + await smallPacer.wait({ files: 1 }); + + const smallElapsed = performance.now() - smallStart; + const bigStart = performance.now(); + + await bigPacer.wait({ files: 1 }); + + const bigElapsed = performance.now() - bigStart; + + // both batches, however oversized, only reserve a single window — not proportionally more + expect(Math.abs(bigElapsed - smallElapsed)).toBeLessThan(windowMs * 1.5); + }); + + it("aborts the wait when the given signal is aborted", async () => { + const pacer = new UploadPacer({ windowMs: 10_000, maxRequestsPerWindow: 1 }); + + await pacer.wait({ requests: 1 }); + + const controller = new AbortController(); + const pending = pacer.wait({ requests: 1 }, controller.signal); + + controller.abort(); + + await expect(pending).rejects.toThrow(); + }); +}); From 5b47e9e5929b4689ee8ea87faf9304a66a577270 Mon Sep 17 00:00:00 2001 From: Todti Date: Thu, 23 Jul 2026 21:06:07 +0100 Subject: [PATCH 2/2] fix(plugin-testops): make upload retry/pacing actually work end-to-end - classifyError only recognized raw AxiosError, but TestOpsClient never sees one: @allurereport/service wraps every HTTP failure into a KnownError/UnknownError first, so retry classification silently fell through to Unknown and withUploadRetry never retried anything for real. - uploadTestResults swallowed its own errors internally, making the retry wrapper around it a no-op; it now lets failures propagate, with logging moved to the plugin-level catch (matching the other upload call sites). - Wire the pacer into every upload call site (global attachments, per-result attachments/fixtures, quality gate), including byte cost for attachment content, not just requests/files. - Add reopenClosedLaunch option: reopens a launch TestOps reports as closed instead of failing the upload outright. - Wrap quality gate upload in withUploadRetry, the one upload path that was missing it. --- packages/plugin-testops/README.md | 2 + packages/plugin-testops/src/client.ts | 135 +++++++++++------- packages/plugin-testops/src/errors.ts | 39 ++++- packages/plugin-testops/src/model.ts | 2 + packages/plugin-testops/src/plugin.ts | 72 ++++++++-- packages/plugin-testops/src/utils/options.ts | 2 + packages/plugin-testops/test/client.test.ts | 64 +++++++++ packages/plugin-testops/test/errors.test.ts | 72 ++++++++++ .../test/features/quality-gate.test.ts | 59 ++++++++ packages/plugin-testops/test/plugin.test.ts | 53 +++++++ packages/plugin-testops/test/utils.ts | 1 + 11 files changed, 438 insertions(+), 63 deletions(-) diff --git a/packages/plugin-testops/README.md b/packages/plugin-testops/README.md index 297cfe1b2d1..9834f6cc68b 100644 --- a/packages/plugin-testops/README.md +++ b/packages/plugin-testops/README.md @@ -80,6 +80,8 @@ The plugin accepts the following options: | `autocloseLaunch` | When `true` (default), the launch is closed automatically when the plugin finishes; set to `false` to keep the launch open | `boolean` | `true` | | `gitFlow` | When `true`, collect Git metadata for TestOps Git Flow on CI uploads (opt-in) | `boolean` | `false` | | `ancestorLimit` | How many ancestor commits to attach to the launch for history linking in TestOps | `number` | `100` | +| `uploadRateLimit` | Caps how fast uploads are sent to TestOps within a rolling time window (requests/files/bytes per window); pass `false` to disable pacing entirely | `{ windowMs: number; maxRequestsPerWindow?: number; maxFilesPerWindow?: number; maxBytesPerWindow?: number } \| false` | `20` req/s, `1000` files/s, `1 GiB`/s | +| `reopenClosedLaunch` | When `true`, a launch that TestOps reports as closed is reopened automatically instead of failing the upload | `boolean` | `false` | ### Using options from environment variables diff --git a/packages/plugin-testops/src/client.ts b/packages/plugin-testops/src/client.ts index c9148327b1e..ea927132302 100644 --- a/packages/plugin-testops/src/client.ts +++ b/packages/plugin-testops/src/client.ts @@ -59,6 +59,20 @@ class TestOpsClientError extends AxiosError<{ const CHUNK_SIZE = 100; const BULK_UPLOAD_CHUNK_SIZE = 1000; +// best-effort: a Buffer/Blob's size is known upfront, a stream's isn't without consuming it, +// so streamed attachments just don't contribute to the byte budget +const attachmentByteLength = (content: AttachmentForUpload["content"]): number => { + if (Buffer.isBuffer(content)) { + return content.length; + } + + if (typeof Blob !== "undefined" && content instanceof Blob) { + return content.size; + } + + return 0; +}; + export class TestOpsClient { #baseUrl: string; #logger = new Logger("TestOpsClient"); @@ -130,6 +144,12 @@ export class TestOpsClient { this.#logger.verbose("Launch closed"); } + async reopenLaunch(launchId: number): Promise { + this.#logger.verbose("Reopening closed launch…"); + await this.#client.post(`/api/launch/${launchId}/reopen`); + this.#logger.verbose("Launch reopened"); + } + async createLaunchCategoriesBulk( launchId: number, items: LaunchCategoryBulkItem[], @@ -343,6 +363,7 @@ export class TestOpsClient { } const formData = new FormData(); + let totalBytes = 0; for (const attachmentLink of attachments) { const attachment = await attachmentsResolver(attachmentLink); @@ -351,12 +372,16 @@ export class TestOpsClient { continue; } + totalBytes += attachmentByteLength(attachment.content); + formData.append("file", attachment.content, { filename: attachment.originalFileName, contentType: attachment.contentType, }); } + await this.#uploadPacer.wait({ requests: 1, files: attachments.length, bytes: totalBytes }); + await this.#client.post("/api/launch/attachment", { body: formData, onUploadProgress(progressEvent) { @@ -378,6 +403,8 @@ export class TestOpsClient { throw new Error("Launch isn't created! Call createLaunch first"); } + await this.#uploadPacer.wait({ requests: 1 }); + await this.#client.post("/api/launch/error/bulk", { body: { launchId: this.#launch.id, @@ -412,53 +439,42 @@ export class TestOpsClient { const uploadedTrs: TestResult[] = []; const envNamesById = new Map(environments.map(({ id, name }) => [id, name])); - try { - for (const trsChunk of trsChunks) { - const chunkEnvs = new Map(); + for (const trsChunk of trsChunks) { + const chunkEnvs = new Map(); - for (const tr of trsChunk) { - const environmentId = tr.environment; + for (const tr of trsChunk) { + const environmentId = tr.environment; - if (environmentId && !this.#namedEnvsIdsByEnv.has(environmentId)) { - chunkEnvs.set(environmentId, { - id: environmentId, - name: envNamesById.get(environmentId) ?? environmentId, - }); - } - } - - if (chunkEnvs.size > 0) { - await this.createNamedEnvs(Array.from(chunkEnvs.values())); + if (environmentId && !this.#namedEnvsIdsByEnv.has(environmentId)) { + chunkEnvs.set(environmentId, { + id: environmentId, + name: envNamesById.get(environmentId) ?? environmentId, + }); } + } - await this.#uploadPacer.wait({ requests: 1, files: trsChunk.length }); + if (chunkEnvs.size > 0) { + await this.createNamedEnvs(Array.from(chunkEnvs.values())); + } - const reportIdsToTestOpsIds = await this.#postTestResultsChunk(trsChunk); + await this.#uploadPacer.wait({ requests: 1, files: trsChunk.length }); - uploadedTrs.push(...trsChunk.filter((tr) => typeof reportIdsToTestOpsIds[tr.id] === "number")); + const reportIdsToTestOpsIds = await this.#postTestResultsChunk(trsChunk); - await this.#uploadChunkAttachmentsAndFixtures( - trsChunk, - reportIdsToTestOpsIds, - attachmentsResolver, - fixturesResolver, - uploadLimitFn, - onProgress, - ); - } + uploadedTrs.push(...trsChunk.filter((tr) => typeof reportIdsToTestOpsIds[tr.id] === "number")); - this.#logger.verbose("Test results upload completed"); - } catch (error) { - if (this.isTestOpsClientError(error)) { - this.#logger.error(`Failed to upload test results: ${error.response?.data.message}`); - this.#logger.debug(error.response.data); - } else if (error instanceof Error) { - this.#logger.error(`Failed to upload test results: ${error.message}`); - } else { - this.#logger.error("Failed to upload test results"); - } + await this.#uploadChunkAttachmentsAndFixtures( + trsChunk, + reportIdsToTestOpsIds, + attachmentsResolver, + fixturesResolver, + uploadLimitFn, + onProgress, + ); } + this.#logger.verbose("Test results upload completed"); + return uploadedTrs; } @@ -525,17 +541,30 @@ export class TestOpsClient { return; } - const attachments = await attachmentsResolver(tr); - const fixtures = (await fixturesResolver(tr)) - .filter((fixture) => validateExecutableName(fixture.name)) - .map((fixture) => ({ - ...fixture, - ...(fixture.steps ? { steps: normalizeTestStepsResults(fixture.steps) } : {}), - })); - - await this.#uploadAttachmentsForResult(testOpsId, attachments as AttachmentForUpload[]); - await this.#uploadFixturesForResult(testOpsId, fixtures); - onProgress?.(); + try { + const attachments = await attachmentsResolver(tr); + const fixtures = (await fixturesResolver(tr)) + .filter((fixture) => validateExecutableName(fixture.name)) + .map((fixture) => ({ + ...fixture, + ...(fixture.steps ? { steps: normalizeTestStepsResults(fixture.steps) } : {}), + })); + + await this.#uploadAttachmentsForResult(testOpsId, attachments as AttachmentForUpload[]); + await this.#uploadFixturesForResult(testOpsId, fixtures); + } catch (error) { + // a subordinate failure (resolver or fixture upload) shouldn't invalidate the test + // result itself, which TestOps has already acknowledged by this point + if (this.isTestOpsClientError(error)) { + this.#logger.error(`Failed to upload fixtures for result ${testOpsId}: ${error.response?.data.message}`); + } else if (error instanceof Error) { + this.#logger.error(`Failed to upload fixtures for result ${testOpsId}: ${error.message}`); + } else { + this.#logger.error(`Failed to upload fixtures for result ${testOpsId}`); + } + } finally { + onProgress?.(); + } }), ), ); @@ -551,7 +580,11 @@ export class TestOpsClient { for (const attachmentsChunk of attachmentsChunks) { const formData = new FormData(); + let chunkBytes = 0; + for (const att of attachmentsChunk) { + chunkBytes += attachmentByteLength(att.content); + formData.append("file", att.content, { filename: att.originalFileName, contentType: att.contentType, @@ -559,6 +592,8 @@ export class TestOpsClient { } try { + await this.#uploadPacer.wait({ requests: 1, files: attachmentsChunk.length, bytes: chunkBytes }); + await this.#client.post(`/api/upload/test-result/${testOpsResultId}/attachment`, { body: formData, headers: formData.getHeaders(), @@ -584,6 +619,8 @@ export class TestOpsClient { const body = toUploadFixturesResultsDto(fixtures); + await this.#uploadPacer.wait({ requests: 1 }); + await this.#client.post(`/api/upload/test-result/${testOpsResultId}/test-fixture-result`, { body, }); @@ -616,6 +653,8 @@ export class TestOpsClient { return item; }); + await this.#uploadPacer.wait({ requests: 1 }); + await this.#client.post("/api/launch/quality-gate/bulk", { body: { launchId: this.#launch.id, diff --git a/packages/plugin-testops/src/errors.ts b/packages/plugin-testops/src/errors.ts index 5f1b8052fa6..309ef6f529d 100644 --- a/packages/plugin-testops/src/errors.ts +++ b/packages/plugin-testops/src/errors.ts @@ -1,5 +1,6 @@ import { setTimeout as delay } from "node:timers/promises"; +import { KnownError, UnknownError } from "@allurereport/service"; import { isAxiosError } from "axios"; export enum ErrorKind { @@ -69,6 +70,21 @@ const classifyHttpStatus = (status: number): ErrorKind => { return ErrorKind.Unknown; }; +export const isClosedLaunchError = (error: unknown): boolean => { + if (isAxiosError(error)) { + return isLaunchClosedMessage(responseMessage(error)); + } + + // TestOpsClient never sees a raw AxiosError: @allurereport/service's createServiceHttpClient + // already unwraps it into a KnownError/UnknownError, formatting the response message into + // `.message` in the process — so the "launch is closed" text still survives there + if (error instanceof KnownError || error instanceof UnknownError) { + return isLaunchClosedMessage(error.message); + } + + return false; +}; + export const classifyError = (error: unknown): ErrorKind => { if (!error) { return ErrorKind.None; @@ -85,6 +101,25 @@ export const classifyError = (error: unknown): ErrorKind => { return status === undefined ? ErrorKind.ServiceTransient : classifyHttpStatus(status); } + // real TestOpsClient calls surface a KnownError (status < 500, response known) or an + // UnknownError (status >= 500, or no response at all) — see the comment on isClosedLaunchError + if (error instanceof KnownError) { + if (isLaunchClosedMessage(error.message)) { + return ErrorKind.ResourceRecoverable; + } + + return typeof error.status === "number" ? classifyHttpStatus(error.status) : ErrorKind.Unknown; + } + + if (error instanceof UnknownError) { + if (isLaunchClosedMessage(error.message)) { + return ErrorKind.ResourceRecoverable; + } + + // UnknownError only ever represents a 5xx response or a request that never got one + return ErrorKind.ServiceTransient; + } + if (error instanceof Error && (error.name === "AbortError" || /timeout/i.test(error.message))) { return ErrorKind.ServiceTransient; } @@ -118,7 +153,7 @@ export type RetryOptions = { maxRetries?: number; baseDelayMs?: number; maxDelayMs?: number; - onRetry?: (error: unknown, attempt: number) => void; + onRetry?: (error: unknown, attempt: number) => void | Promise; }; const DEFAULT_MAX_RETRIES = 3; @@ -148,7 +183,7 @@ export const withUploadRetry = async (operation: () => Promise, options: R } attempt += 1; - onRetry?.(error, attempt); + await onRetry?.(error, attempt); const backoffMs = Math.min(maxDelayMs, baseDelayMs * 2 ** (attempt - 1)); diff --git a/packages/plugin-testops/src/model.ts b/packages/plugin-testops/src/model.ts index b8fd80e41c5..38c72ca4fd0 100644 --- a/packages/plugin-testops/src/model.ts +++ b/packages/plugin-testops/src/model.ts @@ -253,6 +253,8 @@ export type TestOpsUploaderOptions = { filter?: (testResult: TestResult) => boolean; limit?: number; uploadRateLimit?: UploadRateLimit | false; + /** Reopens a launch that TestOps reports as closed instead of failing the upload. Default: false */ + reopenClosedLaunch?: boolean; }; export interface TestOpsFixtureResult extends Omit { diff --git a/packages/plugin-testops/src/plugin.ts b/packages/plugin-testops/src/plugin.ts index 510651d9f0f..939cc9b0608 100644 --- a/packages/plugin-testops/src/plugin.ts +++ b/packages/plugin-testops/src/plugin.ts @@ -15,7 +15,7 @@ import { uniqBy, stubTrue } from "lodash-es"; import { bold } from "yoctocolors"; import { TestOpsClient } from "./client.js"; -import { withUploadRetry } from "./errors.js"; +import { isClosedLaunchError, withUploadRetry } from "./errors.js"; import { LaunchGitFlow, resolveGitFlowOptions } from "./gitFlow/index.js"; import { Logger } from "./logger.js"; import type { TestOpsPluginTestResult, TestOpsPluginOptions, UploadCategory } from "./model.js"; @@ -39,6 +39,7 @@ export class TestOpsPlugin implements Plugin { #launchTags: string[] = []; #uploadedTestResultsIds: Set = new Set(); #autocloseLaunch: boolean = false; + #reopenClosedLaunch: boolean = false; #gitFlow!: LaunchGitFlow; #enabledByConfig: boolean = false; @@ -67,6 +68,7 @@ export class TestOpsPlugin implements Plugin { launchTags, autocloseLaunch = true, uploadRateLimit, + reopenClosedLaunch = false, } = resolvePluginOptions(options); // don't initialize the client when some options are missing @@ -83,6 +85,7 @@ export class TestOpsPlugin implements Plugin { } this.#autocloseLaunch = autocloseLaunch; + this.#reopenClosedLaunch = reopenClosedLaunch; const gitFlowOptions = resolveGitFlowOptions(options); this.#gitFlow = new LaunchGitFlow({ @@ -143,6 +146,25 @@ export class TestOpsPlugin implements Plugin { return true; } + async #reopenLaunchIfClosed(error: unknown): Promise { + if (!this.#reopenClosedLaunch || !isClosedLaunchError(error)) { + return; + } + + const launchId = this.#client.launchId; + + if (launchId === undefined) { + return; + } + + try { + this.#logger.warn(`Launch ${launchId} was closed - reopening before retrying the upload…`); + await this.#client.reopenLaunch(launchId); + } catch (reopenError) { + this.#logger.debug(`Failed to reopen launch ${launchId}: ${reopenError}`); + } + } + async #uploadQualityGateResults(store: AllureStore) { const results = await store.qualityGateResults(); const uniqueResults = uniqBy( @@ -168,12 +190,21 @@ export class TestOpsPlugin implements Plugin { try { progressLogger.log(true); - await this.#client.uploadQualityGateResults(uniqueResults, (percent) => { - if (!completed && percent >= 100) { - completed = true; - progressLogger.increment(); - } - }); + await withUploadRetry( + () => + this.#client.uploadQualityGateResults(uniqueResults, (percent) => { + if (!completed && percent >= 100) { + completed = true; + progressLogger.increment(); + } + }), + { + onRetry: async (error, attempt) => { + this.#logger.debug(`Retrying quality gate results upload (attempt ${attempt}): ${error}`); + await this.#reopenLaunchIfClosed(error); + }, + }, + ); if (!completed) { progressLogger.increment(); @@ -221,8 +252,10 @@ export class TestOpsPlugin implements Plugin { } }), { - onRetry: (error, attempt) => - this.#logger.debug(`Retrying global errors upload (attempt ${attempt}): ${error}`), + onRetry: async (error, attempt) => { + this.#logger.debug(`Retrying global errors upload (attempt ${attempt}): ${error}`); + await this.#reopenLaunchIfClosed(error); + }, }, ); @@ -290,8 +323,10 @@ export class TestOpsPlugin implements Plugin { }, }), { - onRetry: (error, attempt) => - this.#logger.debug(`Retrying global attachments upload (attempt ${attempt}): ${error}`), + onRetry: async (error, attempt) => { + this.#logger.debug(`Retrying global attachments upload (attempt ${attempt}): ${error}`); + await this.#reopenLaunchIfClosed(error); + }, }, ); @@ -347,8 +382,10 @@ export class TestOpsPlugin implements Plugin { onProgress: () => incrementProgress(), }), { - onRetry: (error, attempt) => - this.#logger.debug(`Retrying test results upload (attempt ${attempt}): ${error}`), + onRetry: async (error, attempt) => { + this.#logger.debug(`Retrying test results upload (attempt ${attempt}): ${error}`); + await this.#reopenLaunchIfClosed(error); + }, }, ); @@ -366,6 +403,15 @@ export class TestOpsPlugin implements Plugin { } this.#logger.info(`Uploaded ${uploadedCount} ${uploadedCount > 1 ? "test results" : "test result"}`); + } catch (error) { + if (this.#client.isTestOpsClientError(error)) { + this.#logger.error(`Failed to upload test results: ${error.response.data.message}`); + this.#logger.debug(error.response?.data); + } else if (error instanceof Error) { + this.#logger.error(`Failed to upload test results: ${error.message}`); + } else { + this.#logger.error("Failed to upload test results"); + } } finally { progressLogger.cancel?.(); } diff --git a/packages/plugin-testops/src/utils/options.ts b/packages/plugin-testops/src/utils/options.ts index 9ee82deb522..beb691f5d4a 100644 --- a/packages/plugin-testops/src/utils/options.ts +++ b/packages/plugin-testops/src/utils/options.ts @@ -12,6 +12,7 @@ export const resolvePluginOptions = (options: TestOpsPluginOptions): Omit { }); }); + describe("reopenLaunch", () => { + it("should post to /api/launch/{id}/reopen", async () => { + AxiosMock.post.mockResolvedValue({ data: {} }); + + const client = new TestOpsClient({ + accessToken: fixtures.accessToken, + projectId: fixtures.projectId, + baseUrl: fixtures.endpoint, + }); + + await client.reopenLaunch(fixtures.launch.id); + + expect(AxiosMock.post).toHaveBeenCalledWith( + `/api/launch/${fixtures.launch.id}/reopen`, + undefined, + expect.objectContaining({ + headers: expect.objectContaining({ Authorization: `api-token ${fixtures.accessToken}` }), + }), + ); + }); + }); + describe("launchUrl", () => { it("should return undefined when launch is not created", () => { const client = new TestOpsClient({ @@ -680,6 +704,46 @@ describe("testops http client", () => { }), ); }); + + it("should pace subsequent uploads by the resolved attachments' byte size", async () => { + AxiosMock.post.mockImplementation((url: string) => { + if (url === "/api/launch") { + return Promise.resolve({ data: fixtures.launch }); + } + + if (url === "/api/upload/session") { + return Promise.resolve({ data: { id: 1 } }); + } + + return Promise.resolve({ data: {} }); + }); + + const windowMs = 100; + const client = new TestOpsClient({ + accessToken: fixtures.accessToken, + projectId: fixtures.projectId, + baseUrl: fixtures.endpoint, + uploadRateLimit: { windowMs, maxBytesPerWindow: 1 }, + }); + const attachments = [{ id: "att-1", name: "file.txt", contentType: "text/plain" } as AttachmentLink]; + const attachmentsResolver = vi.fn().mockResolvedValue({ + originalFileName: "file.txt", + contentType: "text/plain", + content: Buffer.from("more than one byte of content"), + }); + + await client.createLaunch(fixtures.launchName, fixtures.launchTags); + await client.createSession(); + + // first call establishes the byte budget usage, well past the 1-byte-per-window limit + await client.uploadGlobalAttachments({ attachments, attachmentsResolver }); + + const start = performance.now(); + + await client.uploadGlobalAttachments({ attachments, attachmentsResolver }); + + expect(performance.now() - start).toBeGreaterThanOrEqual(windowMs - 15); + }); }); describe("uploadGlobalErrors", () => { diff --git a/packages/plugin-testops/test/errors.test.ts b/packages/plugin-testops/test/errors.test.ts index bb48d14b82b..73ed1382c8d 100644 --- a/packages/plugin-testops/test/errors.test.ts +++ b/packages/plugin-testops/test/errors.test.ts @@ -1,9 +1,11 @@ +import { KnownError, UnknownError } from "@allurereport/service"; import { story } from "allure-js-commons"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { ErrorKind, classifyError, + isClosedLaunchError, isPermanentUploadError, isTerminalUploadError, shouldRetryUpload, @@ -63,6 +65,53 @@ describe("classifyError", () => { it("classifies a plain non-axios error as unknown", () => { expect(classifyError(new Error("boom"))).toBe(ErrorKind.Unknown); }); + + // TestOpsClient never sees a raw AxiosError in practice: @allurereport/service's + // createServiceHttpClient wraps every HTTP failure into a KnownError (status < 500) or an + // UnknownError (status >= 500 / no response), so classifyError must recognize those directly. + it("classifies a KnownError by its status code", () => { + expect(classifyError(new KnownError("boom", 401))).toBe(ErrorKind.AuthTerminal); + expect(classifyError(new KnownError("boom", 429))).toBe(ErrorKind.ServiceTransient); + expect(classifyError(new KnownError("boom", 404))).toBe(ErrorKind.NotFoundTerminal); + }); + + it("classifies a KnownError with a 'launch is closed' message as resource recoverable regardless of status", () => { + expect(classifyError(new KnownError("Allure service request failed: Launch is closed", 400))).toBe( + ErrorKind.ResourceRecoverable, + ); + }); + + it("classifies a statusless KnownError as unknown", () => { + expect(classifyError(new KnownError("boom"))).toBe(ErrorKind.Unknown); + }); + + it("classifies an UnknownError as service transient (it only ever means 5xx or no response)", () => { + expect(classifyError(new UnknownError("boom"))).toBe(ErrorKind.ServiceTransient); + }); + + it("classifies an UnknownError with a 'launch is closed' message as resource recoverable", () => { + expect(classifyError(new UnknownError("Allure service request failed: closed launch"))).toBe( + ErrorKind.ResourceRecoverable, + ); + }); +}); + +describe("isClosedLaunchError", () => { + it("recognizes a 'launch is closed' message regardless of status", () => { + expect(isClosedLaunchError(axiosError(400, "Launch is closed"))).toBe(true); + expect(isClosedLaunchError(axiosError(423, "closed launch"))).toBe(true); + }); + + it("does not flag unrelated errors", () => { + expect(isClosedLaunchError(axiosError(500, "internal error"))).toBe(false); + expect(isClosedLaunchError(new Error("boom"))).toBe(false); + }); + + it("recognizes a 'launch is closed' message wrapped in a KnownError or UnknownError", () => { + expect(isClosedLaunchError(new KnownError("Allure service request failed: Launch is closed", 400))).toBe(true); + expect(isClosedLaunchError(new UnknownError("Allure service request failed: closed launch"))).toBe(true); + expect(isClosedLaunchError(new KnownError("Allure service request failed: not found", 404))).toBe(false); + }); }); describe("shouldRetryUpload / isPermanentUploadError", () => { @@ -125,4 +174,27 @@ describe("withUploadRetry", () => { await expect(withUploadRetry(operation, { baseDelayMs: 0, maxRetries: 2 })).rejects.toEqual(axiosError(503)); expect(operation).toHaveBeenCalledTimes(3); }); + + it("awaits an async onRetry before the next attempt", async () => { + const order: string[] = []; + const onRetry = vi.fn().mockImplementation(async () => { + order.push("onRetry:start"); + await Promise.resolve(); + order.push("onRetry:end"); + }); + const operation = vi + .fn() + .mockImplementationOnce(async () => { + throw axiosError(503); + }) + .mockImplementationOnce(async () => { + order.push("operation:retry"); + return "ok"; + }); + + const result = await withUploadRetry(operation, { baseDelayMs: 0, onRetry }); + + expect(result).toBe("ok"); + expect(order).toEqual(["onRetry:start", "onRetry:end", "operation:retry"]); + }); }); diff --git a/packages/plugin-testops/test/features/quality-gate.test.ts b/packages/plugin-testops/test/features/quality-gate.test.ts index e49cbb980c9..fe4592410f4 100644 --- a/packages/plugin-testops/test/features/quality-gate.test.ts +++ b/packages/plugin-testops/test/features/quality-gate.test.ts @@ -170,4 +170,63 @@ describe("Quality Gate upload", () => { expect(uploadQualityGateResultsRequest).not.toHaveBeenCalled(); }); + + test("retries quality gate upload once after a transient failure", async () => { + const { uploadQualityGateResultsRequest } = mockRequests(); + + uploadQualityGateResultsRequest.mockRejectedValueOnce({ + isAxiosError: true, + response: { status: 503, data: {} }, + }); + + const plugin = new TestOpsPlugin({ + accessToken: "test", + endpoint: "http://example.com", + projectId: "12345", + launchName: "Test Launch", + launchTags: [], + autocloseLaunch: true, + filter: () => true, + limit: 10, + }); + + const store = mockAllureStore(); + + store.allTestResults.mockResolvedValue([ + { + id: "tr-1", + name: "Sample test", + fullName: "suite Sample test", + status: "passed", + stage: "finished", + start: 1, + stop: 2, + steps: [], + } as unknown as TestResult, + ]); + store.qualityGateResults.mockResolvedValue([ + { + rule: "coverage", + message: "Coverage below threshold", + success: false, + } as QualityGateValidationResult, + ]); + + await plugin.start( + { + allureVersion: "3.0.0", + reportUuid: "test-uuid", + reportName: "Test Report", + output: "/tmp/out", + categories: [], + publish: true, + id: "test", + reportFiles: [] as any, + state: {} as PluginState, + }, + store as unknown as AllureStore, + ); + + expect(uploadQualityGateResultsRequest).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/plugin-testops/test/plugin.test.ts b/packages/plugin-testops/test/plugin.test.ts index 86aecdc611b..77a1f4428de 100644 --- a/packages/plugin-testops/test/plugin.test.ts +++ b/packages/plugin-testops/test/plugin.test.ts @@ -958,6 +958,59 @@ describe("testops plugin", () => { expect(TestOpsClientMock.prototype.uploadTestResults).toHaveBeenCalledTimes(1); }); }); + + describe("reopenClosedLaunch", () => { + const closedLaunchError = { + isAxiosError: true, + response: { status: 423, data: { message: "Launch is closed" } }, + }; + + it("reopens the launch and retries when the option is enabled", async () => { + (resolvePluginOptions as Mock).mockReturnValue({ + accessToken: fixtures.accessToken, + endpoint: fixtures.endpoint, + projectId: fixtures.projectId, + launchName: "Allure Report", + launchTags: [], + reopenClosedLaunch: true, + }); + plugin = new TestOpsPlugin({} as TestOpsPluginOptions); + + AllureStoreMock.prototype.allTestResults.mockResolvedValue(fixtures.testResults.slice(0, 1)); + AllureStoreMock.prototype.attachmentsByTrId.mockResolvedValue([]); + AllureStoreMock.prototype.attachmentContentById.mockResolvedValue(fixtures.attachmentContent); + AllureStoreMock.prototype.fixturesByTrId.mockResolvedValue([]); + TestOpsClientMock.prototype.uploadTestResults.mockRejectedValueOnce(closedLaunchError); + + await plugin.start({ reportName: "Test Launch" } as PluginContext, store); + + expect(TestOpsClientMock.prototype.reopenLaunch).toHaveBeenCalledWith(TestOpsClientMock.prototype.launchId); + expect(TestOpsClientMock.prototype.uploadTestResults).toHaveBeenCalledTimes(2); + }); + + it("does not reopen the launch when the option is disabled (default)", async () => { + (resolvePluginOptions as Mock).mockReturnValue({ + accessToken: fixtures.accessToken, + endpoint: fixtures.endpoint, + projectId: fixtures.projectId, + launchName: "Allure Report", + launchTags: [], + }); + plugin = new TestOpsPlugin({} as TestOpsPluginOptions); + + AllureStoreMock.prototype.allTestResults.mockResolvedValue(fixtures.testResults.slice(0, 1)); + AllureStoreMock.prototype.attachmentsByTrId.mockResolvedValue([]); + AllureStoreMock.prototype.attachmentContentById.mockResolvedValue(fixtures.attachmentContent); + AllureStoreMock.prototype.fixturesByTrId.mockResolvedValue([]); + TestOpsClientMock.prototype.uploadTestResults.mockRejectedValueOnce(closedLaunchError); + + await plugin.start({ reportName: "Test Launch" } as PluginContext, store); + + expect(TestOpsClientMock.prototype.reopenLaunch).not.toHaveBeenCalled(); + // still retries: "launch is closed" is a resource-recoverable error kind regardless of the reopen option + expect(TestOpsClientMock.prototype.uploadTestResults).toHaveBeenCalledTimes(2); + }); + }); }); describe("when client is not initialized", () => { diff --git a/packages/plugin-testops/test/utils.ts b/packages/plugin-testops/test/utils.ts index 620b87759e5..f524fef7bd3 100644 --- a/packages/plugin-testops/test/utils.ts +++ b/packages/plugin-testops/test/utils.ts @@ -22,6 +22,7 @@ TestOpsClientMock.prototype = { stopUpload: vi.fn(), createLaunchCategoriesBulk: vi.fn().mockResolvedValue([]), closeLaunch: vi.fn(), + reopenLaunch: vi.fn(), getNamedEnvFor: vi.fn().mockReturnValue(undefined), };