diff --git a/packages/probitas-runner/errors.ts b/packages/probitas-runner/errors.ts index dc0b4e6..b1143f8 100644 --- a/packages/probitas-runner/errors.ts +++ b/packages/probitas-runner/errors.ts @@ -31,35 +31,128 @@ export class StepTimeoutError extends Error { this.attemptNumber = attemptNumber; this.elapsedMs = elapsedMs; } + + /** + * Creates an AbortSignal that aborts with StepTimeoutError after the specified timeout. + * + * @param timeout - Timeout duration in milliseconds + * @param params - Step information for the error + * @returns AbortSignal with Disposable interface for cleanup + */ + static timeoutSignal( + timeout: number, + params: { + stepName: string; + attemptNumber: number; + }, + ): AbortSignal & Disposable { + const controller = new AbortController(); + const startTime = performance.now(); + const timeoutId = setTimeout(() => { + const elapsedMs = Math.round(performance.now() - startTime); + const error = new StepTimeoutError( + params.stepName, + timeout, + params.attemptNumber, + elapsedMs, + ); + controller.abort(error); + }, timeout); + + // Cleanup timeout when signal is aborted by other means + controller.signal.addEventListener("abort", () => { + clearTimeout(timeoutId); + }, { once: true }); + + // Add Disposable interface for manual cleanup + return Object.assign(controller.signal, { + [Symbol.dispose]: () => { + clearTimeout(timeoutId); + }, + }); + } } /** * Error thrown when a scenario execution times out. * * Contains structured information about the timeout including - * the scenario name, timeout duration, and total elapsed time. + * the scenario name, timeout duration, total elapsed time, + * and optionally the step being executed when timeout occurred. */ export class ScenarioTimeoutError extends Error { override readonly name = "ScenarioTimeoutError"; readonly scenarioName: string; readonly timeoutMs: number; readonly elapsedMs: number; + readonly currentStepName?: string; + readonly currentStepIndex?: number; constructor( scenarioName: string, timeoutMs: number, elapsedMs: number, - options?: ErrorOptions, + options?: ErrorOptions & { + currentStepName?: string; + currentStepIndex?: number; + }, ) { + const stepMsg = options?.currentStepName + ? ` while executing step "${options.currentStepName}"${ + options.currentStepIndex !== undefined + ? ` (step ${options.currentStepIndex + 1})` + : "" + }` + : ""; const elapsedMsg = elapsedMs !== timeoutMs ? `, total elapsed: ${elapsedMs}ms` : ""; super( - `Scenario "${scenarioName}" timed out after ${timeoutMs}ms${elapsedMsg}`, + `Scenario "${scenarioName}" timed out after ${timeoutMs}ms${stepMsg}${elapsedMsg}`, options, ); this.scenarioName = scenarioName; this.timeoutMs = timeoutMs; this.elapsedMs = elapsedMs; + this.currentStepName = options?.currentStepName; + this.currentStepIndex = options?.currentStepIndex; + } + + /** + * Creates an AbortSignal that aborts with ScenarioTimeoutError after the specified timeout. + * + * @param timeout - Timeout duration in milliseconds + * @param params - Scenario information for the error + * @returns AbortSignal with Disposable interface for cleanup + */ + static timeoutSignal( + timeout: number, + params: { + scenarioName: string; + }, + ): AbortSignal & Disposable { + const controller = new AbortController(); + const startTime = performance.now(); + const timeoutId = setTimeout(() => { + const elapsedMs = Math.round(performance.now() - startTime); + const error = new ScenarioTimeoutError( + params.scenarioName, + timeout, + elapsedMs, + ); + controller.abort(error); + }, timeout); + + // Cleanup timeout when signal is aborted by other means + controller.signal.addEventListener("abort", () => { + clearTimeout(timeoutId); + }, { once: true }); + + // Add Disposable interface for manual cleanup + return Object.assign(controller.signal, { + [Symbol.dispose]: () => { + clearTimeout(timeoutId); + }, + }); } } diff --git a/packages/probitas-runner/errors_test.ts b/packages/probitas-runner/errors_test.ts new file mode 100644 index 0000000..aeefedb --- /dev/null +++ b/packages/probitas-runner/errors_test.ts @@ -0,0 +1,166 @@ +import { expect } from "@std/expect"; +import { delay } from "@std/async/delay"; +import { ScenarioTimeoutError, StepTimeoutError } from "./errors.ts"; + +Deno.test("ScenarioTimeoutError - constructor creates error with correct properties", () => { + const error = new ScenarioTimeoutError("test-scenario", 5000, 5100); + + expect(error.name).toBe("ScenarioTimeoutError"); + expect(error.scenarioName).toBe("test-scenario"); + expect(error.timeoutMs).toBe(5000); + expect(error.elapsedMs).toBe(5100); + expect(error.currentStepName).toBeUndefined(); + expect(error.currentStepIndex).toBeUndefined(); + expect(error.message).toBe( + 'Scenario "test-scenario" timed out after 5000ms, total elapsed: 5100ms', + ); +}); + +Deno.test("ScenarioTimeoutError - constructor with step information", () => { + const error = new ScenarioTimeoutError("test-scenario", 5000, 5100, { + currentStepName: "slow-step", + currentStepIndex: 2, + }); + + expect(error.currentStepName).toBe("slow-step"); + expect(error.currentStepIndex).toBe(2); + expect(error.message).toBe( + 'Scenario "test-scenario" timed out after 5000ms while executing step "slow-step" (step 3), total elapsed: 5100ms', + ); +}); + +Deno.test("ScenarioTimeoutError - constructor with only step name", () => { + const error = new ScenarioTimeoutError("test-scenario", 5000, 5000, { + currentStepName: "slow-step", + }); + + expect(error.currentStepName).toBe("slow-step"); + expect(error.currentStepIndex).toBeUndefined(); + expect(error.message).toBe( + 'Scenario "test-scenario" timed out after 5000ms while executing step "slow-step"', + ); +}); + +Deno.test("ScenarioTimeoutError.timeoutSignal - creates signal that aborts after timeout", async () => { + using signal = ScenarioTimeoutError.timeoutSignal(50, { + scenarioName: "test-scenario", + }); + + expect(signal.aborted).toBe(false); + + await delay(100); + + expect(signal.aborted).toBe(true); + expect(signal.reason).toBeInstanceOf(ScenarioTimeoutError); + + const error = signal.reason as ScenarioTimeoutError; + expect(error.scenarioName).toBe("test-scenario"); + expect(error.timeoutMs).toBe(50); + expect(error.elapsedMs).toBeGreaterThanOrEqual(50); + expect(error.elapsedMs).toBeLessThan(200); +}); + +Deno.test("ScenarioTimeoutError.timeoutSignal - cleanup via dispose", async () => { + const signal = ScenarioTimeoutError.timeoutSignal(1000, { + scenarioName: "test-scenario", + }); + + expect(signal.aborted).toBe(false); + + // Dispose immediately to cleanup timer + signal[Symbol.dispose](); + + await delay(50); + + // Should not abort because timer was cleaned up + expect(signal.aborted).toBe(false); +}); + +Deno.test("ScenarioTimeoutError.timeoutSignal - cleanup via using declaration", async () => { + { + using signal = ScenarioTimeoutError.timeoutSignal(1000, { + scenarioName: "test-scenario", + }); + + expect(signal.aborted).toBe(false); + } // signal is disposed here + + await delay(50); + + // Timer should be cleaned up, no leak +}); + +Deno.test("StepTimeoutError - constructor creates error with correct properties", () => { + const error = new StepTimeoutError("test-step", 3000, 2, 3200); + + expect(error.name).toBe("StepTimeoutError"); + expect(error.stepName).toBe("test-step"); + expect(error.timeoutMs).toBe(3000); + expect(error.attemptNumber).toBe(2); + expect(error.elapsedMs).toBe(3200); + expect(error.message).toBe( + 'Step "test-step" timed out after 3000ms (attempt 2), total elapsed: 3200ms', + ); +}); + +Deno.test("StepTimeoutError - constructor for first attempt", () => { + const error = new StepTimeoutError("test-step", 3000, 1, 3000); + + expect(error.attemptNumber).toBe(1); + expect(error.message).toBe( + 'Step "test-step" timed out after 3000ms', + ); +}); + +Deno.test("StepTimeoutError.timeoutSignal - creates signal that aborts after timeout", async () => { + using signal = StepTimeoutError.timeoutSignal(50, { + stepName: "test-step", + attemptNumber: 1, + }); + + expect(signal.aborted).toBe(false); + + await delay(100); + + expect(signal.aborted).toBe(true); + expect(signal.reason).toBeInstanceOf(StepTimeoutError); + + const error = signal.reason as StepTimeoutError; + expect(error.stepName).toBe("test-step"); + expect(error.timeoutMs).toBe(50); + expect(error.attemptNumber).toBe(1); + expect(error.elapsedMs).toBeGreaterThanOrEqual(50); + expect(error.elapsedMs).toBeLessThan(200); +}); + +Deno.test("StepTimeoutError.timeoutSignal - cleanup via dispose", async () => { + const signal = StepTimeoutError.timeoutSignal(1000, { + stepName: "test-step", + attemptNumber: 1, + }); + + expect(signal.aborted).toBe(false); + + // Dispose immediately to cleanup timer + signal[Symbol.dispose](); + + await delay(50); + + // Should not abort because timer was cleaned up + expect(signal.aborted).toBe(false); +}); + +Deno.test("StepTimeoutError.timeoutSignal - measures elapsed time correctly", async () => { + using signal = StepTimeoutError.timeoutSignal(50, { + stepName: "test-step", + attemptNumber: 1, + }); + + await delay(100); + + const error = signal.reason as StepTimeoutError; + // Elapsed time should be approximately 50ms (actual timeout duration) + // with some tolerance for timer precision + expect(error.elapsedMs).toBeGreaterThanOrEqual(50); + expect(error.elapsedMs).toBeLessThan(150); +}); diff --git a/packages/probitas-runner/runner.ts b/packages/probitas-runner/runner.ts index e70d394..7990b3c 100644 --- a/packages/probitas-runner/runner.ts +++ b/packages/probitas-runner/runner.ts @@ -10,7 +10,8 @@ import { ScenarioRunner } from "./scenario_runner.ts"; import { toScenarioMetadata } from "./metadata.ts"; import { timeit } from "./utils/timeit.ts"; import { mergeSignals } from "./utils/signal.ts"; -import { ScenarioTimeoutError, StepTimeoutError } from "./errors.ts"; +import { ScenarioTimeoutError } from "./errors.ts"; +import { Skip } from "./skip.ts"; /** * Top-level test runner that orchestrates execution of multiple scenarios. @@ -69,7 +70,6 @@ export class Runner { // Create abort controller for outer context const controller = new AbortController(); - const { signal } = controller; options?.signal?.addEventListener("abort", () => { // Pass the reason from external signal to internal controller controller.abort(options.signal?.reason); @@ -88,7 +88,7 @@ export class Runner { maxConcurrency, maxFailures, timeout, - signal, + controller, options?.stepOptions, ) ); @@ -118,13 +118,16 @@ export class Runner { timeout: number, signal?: AbortSignal, ): Promise { - const timeoutSignal = mergeSignals( - signal, - AbortSignal.timeout(timeout), - ); + // Create timeout signal that aborts with ScenarioTimeoutError + using timeoutSignal = ScenarioTimeoutError.timeoutSignal(timeout, { + scenarioName: scenario.name, + }); + + // Merge with external signal + const mergedSignal = mergeSignals(signal, timeoutSignal); const result = await timeit(() => - scenarioRunner.run(scenario, { signal: timeoutSignal }) + scenarioRunner.run(scenario, { signal: mergedSignal }) ); // Handle timeit result @@ -133,25 +136,7 @@ export class Runner { throw result.error; } - const scenarioResult = result.value; - - // Check if scenario failed due to timeout - if ( - scenarioResult.status === "failed" && - isTimeoutError(scenarioResult.error, timeoutSignal) - ) { - return { - ...scenarioResult, - error: new ScenarioTimeoutError( - scenario.name, - timeout, - result.duration, - { cause: scenarioResult.error }, - ), - }; - } - - return scenarioResult; + return result.value; } async #run( @@ -160,20 +145,43 @@ export class Runner { maxConcurrency: number, maxFailures: number, timeout: number, - signal?: AbortSignal, + controller: AbortController, stepOptions?: StepOptions, ): Promise { // Parallel execution with concurrency control // maxConcurrency=1 means sequential execution const concurrency = maxConcurrency || scenarios.length; const scenarioRunner = new ScenarioRunner(this.reporter, stepOptions); + const { signal } = controller; let failureCount = 0; + for (const batch of chunk(scenarios, concurrency)) { - signal?.throwIfAborted(); + // Don't throw - just skip remaining batches if aborted + if (signal.aborted) { + break; + } + await Promise.all( batch.map(async (scenario: ScenarioDefinition) => { - signal?.throwIfAborted(); + // Check if already aborted (by maxFailures or external signal) + if (signal.aborted) { + const skipResult: ScenarioResult = { + status: "skipped", + metadata: toScenarioMetadata(scenario), + duration: 0, + steps: [], + error: signal.reason ?? + new Skip("Skipped due to previous failures"), + }; + scenarioResults.push(skipResult); + await this.reporter.onScenarioStart?.(skipResult.metadata); + await this.reporter.onScenarioEnd?.( + skipResult.metadata, + skipResult, + ); + return; + } // Execute scenario with optional timeout // Priority: scenario timeout > RunOptions timeout @@ -188,46 +196,35 @@ export class Runner { : await scenarioRunner.run(scenario, { signal }); scenarioResults.push(scenarioResult); + + // Check if we've reached maxFailures - abort all remaining scenarios if (scenarioResult.status === "failed") { failureCount++; if (maxFailures !== 0 && failureCount >= maxFailures) { - throw scenarioResult.error; + controller.abort(new Skip("Skipped due to previous failures")); } } }), ); } - } -} - -function isTimeoutError(error: unknown, signal?: AbortSignal): boolean { - // Direct TimeoutError - if (error instanceof DOMException && error.name === "TimeoutError") { - return true; - } - - // StepTimeoutError - if (error instanceof StepTimeoutError) { - return true; - } - // AbortError caused by timeout - if ( - error instanceof DOMException && - error.name === "AbortError" && - signal?.aborted && - signal.reason instanceof DOMException && - signal.reason.name === "TimeoutError" - ) { - return true; + // Add skip results for any remaining scenarios that weren't executed + // This handles the case where we broke out of the batch loop early + const executedCount = scenarioResults.length; + if (executedCount < scenarios.length) { + for (let i = executedCount; i < scenarios.length; i++) { + const scenario = scenarios[i]; + const skipResult: ScenarioResult = { + status: "skipped", + metadata: toScenarioMetadata(scenario), + duration: 0, + steps: [], + error: signal.reason ?? new Skip("Skipped due to previous failures"), + }; + scenarioResults.push(skipResult); + await this.reporter.onScenarioStart?.(skipResult.metadata); + await this.reporter.onScenarioEnd?.(skipResult.metadata, skipResult); + } + } } - - return false; } - -/** - * @internal - */ -export const _internal = { - isTimeoutError, -}; diff --git a/packages/probitas-runner/runner_test.ts b/packages/probitas-runner/runner_test.ts index b52462d..0be9684 100644 --- a/packages/probitas-runner/runner_test.ts +++ b/packages/probitas-runner/runner_test.ts @@ -7,29 +7,12 @@ import { TestReporter, } from "./_testutils.ts"; import { Skip } from "./skip.ts"; -import { _internal, Runner } from "./runner.ts"; +import { Runner } from "./runner.ts"; import { createScopedSignal } from "./utils/signal.ts"; import { ScenarioTimeoutError } from "./errors.ts"; const reporter = new TestReporter(); -Deno.test("isTimeoutError detects TimeoutError", () => { - const error = new DOMException("Timeout", "TimeoutError"); - expect(_internal.isTimeoutError(error)).toBe(true); -}); - -Deno.test("isTimeoutError should detect AbortError caused by timeout", async () => { - const signal = AbortSignal.timeout(10); - - // Wait for signal to abort - await delay(20); - - const error = new DOMException("Aborted", "AbortError"); - - // AbortError should be detected as timeout error when signal.reason is TimeoutError - expect(_internal.isTimeoutError(error, signal)).toBe(true); -}); - Deno.test("Runner run runs single scenario", async () => { const runner = new Runner(reporter); const scenarios = [ @@ -158,7 +141,7 @@ Deno.test("Runner run runs multiple scenarios (skip)", async () => { }); }); -Deno.test("Runner run runs multiple scenarios (failed)", async () => { +Deno.test("Runner run runs multiple scenarios (failed) - aborts on maxFailures", async () => { using signal = createScopedSignal(); const runner = new Runner(reporter); const scenarios = [ @@ -167,8 +150,8 @@ Deno.test("Runner run runs multiple scenarios (failed)", async () => { steps: [ createTestStep({ name: "Step 1", - fn: async () => { - await delay(0, { signal }); + fn: async (ctx) => { + await delay(0, { signal: ctx.signal }); return "1"; }, }), @@ -179,8 +162,8 @@ Deno.test("Runner run runs multiple scenarios (failed)", async () => { steps: [ createTestStep({ name: "Step 1", - fn: async () => { - await delay(50, { signal }); + fn: async (ctx) => { + await delay(50, { signal: ctx.signal }); throw "2"; }, }), @@ -191,8 +174,8 @@ Deno.test("Runner run runs multiple scenarios (failed)", async () => { steps: [ createTestStep({ name: "Step 1", - fn: async () => { - await delay(100, { signal }); + fn: async (ctx) => { + await delay(100, { signal: ctx.signal }); return "3"; }, }), @@ -204,12 +187,219 @@ Deno.test("Runner run runs multiple scenarios (failed)", async () => { signal, maxFailures: 1, }); + + // When maxFailures is reached, all scenarios (running or pending) should be aborted + // Scenario 1 passes (fastest, 0ms delay) + // Scenario 2 fails (50ms delay, triggers maxFailures) + // Scenario 3 should be aborted/skipped (100ms delay, still running when maxFailures reached) expect(summary).toMatchObject({ total: 3, passed: 1, - skipped: 1, + skipped: 1, // Scenario 3 aborted + failed: 1, // Scenario 2 + }); +}); + +Deno.test("Runner run with maxFailures skips remaining scenarios (sequential)", async () => { + const runner = new Runner(reporter); + const executionOrder: string[] = []; + const scenarios = [ + createTestScenario({ + name: "Scenario 1", + steps: [ + createTestStep({ + name: "Step 1", + fn: () => { + executionOrder.push("Scenario 1"); + return "1"; + }, + }), + ], + }), + createTestScenario({ + name: "Scenario 2", + steps: [ + createTestStep({ + name: "Step 1", + fn: () => { + executionOrder.push("Scenario 2"); + throw new Error("Scenario 2 failed"); + }, + }), + ], + }), + createTestScenario({ + name: "Scenario 3", + steps: [ + createTestStep({ + name: "Step 1", + fn: () => { + executionOrder.push("Scenario 3"); + return "3"; + }, + }), + ], + }), + createTestScenario({ + name: "Scenario 4", + steps: [ + createTestStep({ + name: "Step 1", + fn: () => { + executionOrder.push("Scenario 4"); + return "4"; + }, + }), + ], + }), + ]; + + const summary = await runner.run(scenarios, { + maxConcurrency: 1, // Sequential execution + maxFailures: 1, + }); + + // Scenario 1 should pass, Scenario 2 should fail, Scenario 3 and 4 should be skipped + expect(summary).toMatchObject({ + total: 4, + passed: 1, failed: 1, + skipped: 2, + }); + + // Only Scenario 1 and 2 should have been executed + expect(executionOrder).toEqual(["Scenario 1", "Scenario 2"]); + + // Check scenario results + const results = summary.scenarios; + expect(results.length).toBe(4); + expect(results[0].status).toBe("passed"); + expect(results[0].metadata.name).toBe("Scenario 1"); + expect(results[1].status).toBe("failed"); + expect(results[1].metadata.name).toBe("Scenario 2"); + expect(results[2].status).toBe("skipped"); + expect(results[2].metadata.name).toBe("Scenario 3"); + expect(results[3].status).toBe("skipped"); + expect(results[3].metadata.name).toBe("Scenario 4"); +}); + +Deno.test("Runner run with maxFailures aborts in-progress scenarios (parallel)", async () => { + using signal = createScopedSignal(); + const runner = new Runner(reporter); + const executionOrder: string[] = []; + const scenarios = [ + createTestScenario({ + name: "Scenario 1", + steps: [ + createTestStep({ + name: "Step 1", + fn: async (ctx) => { + executionOrder.push("Scenario 1 start"); + try { + await delay(100, { signal: ctx.signal }); // Slow - will be aborted + executionOrder.push("Scenario 1 end"); + return "1"; + } catch (err) { + executionOrder.push("Scenario 1 aborted"); + throw err; + } + }, + }), + ], + }), + createTestScenario({ + name: "Scenario 2", + steps: [ + createTestStep({ + name: "Step 1", + fn: async (ctx) => { + executionOrder.push("Scenario 2 start"); + await delay(10, { signal: ctx.signal }); // Fast + executionOrder.push("Scenario 2 end"); + throw new Error("Scenario 2 failed"); + }, + }), + ], + }), + createTestScenario({ + name: "Scenario 3", + steps: [ + createTestStep({ + name: "Step 1", + fn: async (ctx) => { + executionOrder.push("Scenario 3 start"); + try { + await delay(100, { signal: ctx.signal }); // Slow - will be aborted + executionOrder.push("Scenario 3 end"); + return "3"; + } catch (err) { + executionOrder.push("Scenario 3 aborted"); + throw err; + } + }, + }), + ], + }), + createTestScenario({ + name: "Scenario 4", + steps: [ + createTestStep({ + name: "Step 1", + fn: () => { + executionOrder.push("Scenario 4 start"); + return "4"; + }, + }), + ], + }), + ]; + + const summary = await runner.run(scenarios, { + signal, + maxConcurrency: 3, // Scenarios 1-3 run in parallel, 4 is in next batch + maxFailures: 1, + }); + + // Scenario 1, 2, 3 all start in parallel + // Scenario 2 finishes first and fails, triggers maxFailures + // Scenarios 1, 3 are aborted immediately (signal.abort called) + // Scenario 4 is in the next batch, never starts + expect(summary).toMatchObject({ + total: 4, + passed: 0, + failed: 1, // Scenario 2 + skipped: 3, // Scenarios 1, 3, 4 all skipped }); + + // Verify execution order + expect(executionOrder).toContain("Scenario 1 start"); + expect(executionOrder).toContain("Scenario 1 aborted"); // Aborted, not completed + expect(executionOrder).not.toContain("Scenario 1 end"); + + expect(executionOrder).toContain("Scenario 2 start"); + expect(executionOrder).toContain("Scenario 2 end"); + + expect(executionOrder).toContain("Scenario 3 start"); + expect(executionOrder).toContain("Scenario 3 aborted"); // Aborted, not completed + expect(executionOrder).not.toContain("Scenario 3 end"); + + // Scenario 4 should never start + expect(executionOrder).not.toContain("Scenario 4 start"); + + // Check scenario results + const results = summary.scenarios; + expect(results.length).toBe(4); + + // Find scenarios by name since order may vary in parallel execution + const scenario1 = results.find((r) => r.metadata.name === "Scenario 1")!; + const scenario2 = results.find((r) => r.metadata.name === "Scenario 2")!; + const scenario3 = results.find((r) => r.metadata.name === "Scenario 3")!; + const scenario4 = results.find((r) => r.metadata.name === "Scenario 4")!; + + expect(scenario1.status).toBe("skipped"); + expect(scenario2.status).toBe("failed"); + expect(scenario3.status).toBe("skipped"); + expect(scenario4.status).toBe("skipped"); }); Deno.test({ @@ -453,3 +643,104 @@ Deno.test({ } }, }); + +Deno.test({ + name: "Runner run with timeout includes step info in ScenarioTimeoutError", + sanitizeResources: false, + sanitizeOps: false, + async fn() { + const runner = new Runner(reporter); + const scenarios = [ + createTestScenario({ + name: "Timeout with step info", + steps: [ + createTestStep({ + name: "fast-step", + fn: async (ctx) => { + await delay(10, { signal: ctx.signal }); + return "done"; + }, + }), + createTestStep({ + name: "slow-step", + fn: async (ctx) => { + await delay(200, { signal: ctx.signal }); + return "should-timeout"; + }, + }), + createTestStep({ + name: "never-reached", + fn: () => "not-reached", + }), + ], + }), + ]; + + const summary = await runner.run(scenarios, { + timeout: 50, + }); + + expect(summary).toMatchObject({ + total: 1, + passed: 0, + skipped: 0, + failed: 1, + }); + + const scenarioResult = summary.scenarios[0]; + expect(scenarioResult.status).toBe("failed"); + if (scenarioResult.status === "failed") { + expect(scenarioResult.error).toBeInstanceOf(ScenarioTimeoutError); + const timeoutError = scenarioResult.error as ScenarioTimeoutError; + expect(timeoutError.scenarioName).toBe("Timeout with step info"); + expect(timeoutError.timeoutMs).toBe(50); + expect(timeoutError.currentStepName).toBe("slow-step"); + expect(timeoutError.currentStepIndex).toBe(1); + expect(timeoutError.message).toContain( + 'while executing step "slow-step"', + ); + expect(timeoutError.message).toContain("(step 2)"); + } + }, +}); + +Deno.test({ + name: "Runner run with timeout on first step includes step index 0", + sanitizeResources: false, + sanitizeOps: false, + async fn() { + const runner = new Runner(reporter); + const scenarios = [ + createTestScenario({ + name: "First step timeout", + steps: [ + createTestStep({ + name: "first-slow-step", + fn: async (ctx) => { + await delay(200, { signal: ctx.signal }); + return "should-timeout"; + }, + }), + createTestStep({ + name: "never-reached", + fn: () => "not-reached", + }), + ], + }), + ]; + + const summary = await runner.run(scenarios, { + timeout: 50, + }); + + const scenarioResult = summary.scenarios[0]; + expect(scenarioResult.status).toBe("failed"); + if (scenarioResult.status === "failed") { + expect(scenarioResult.error).toBeInstanceOf(ScenarioTimeoutError); + const timeoutError = scenarioResult.error as ScenarioTimeoutError; + expect(timeoutError.currentStepName).toBe("first-slow-step"); + expect(timeoutError.currentStepIndex).toBe(0); + expect(timeoutError.message).toContain("(step 1)"); + } + }, +}); diff --git a/packages/probitas-runner/scenario_runner.ts b/packages/probitas-runner/scenario_runner.ts index 11e9430..35b9bdc 100644 --- a/packages/probitas-runner/scenario_runner.ts +++ b/packages/probitas-runner/scenario_runner.ts @@ -5,6 +5,7 @@ import { StepRunner } from "./step_runner.ts"; import { toScenarioMetadata } from "./metadata.ts"; import { timeit } from "./utils/timeit.ts"; import { createScenarioContext } from "./context.ts"; +import { ScenarioTimeoutError } from "./errors.ts"; export interface RunOptions { readonly signal?: AbortSignal; @@ -61,14 +62,33 @@ export class ScenarioRunner { this.#stepOptions, ); - for (const step of scenario.steps) { + for (let stepIndex = 0; stepIndex < scenario.steps.length; stepIndex++) { + const step = scenario.steps[stepIndex]; + signal?.throwIfAborted(); - const stepResult = await stepRunner.run(step, stack); - stepResults.push(stepResult); + try { + const stepResult = await stepRunner.run(step, stack); + stepResults.push(stepResult); - if (stepResult.status !== "passed") { - throw stepResult.error; + if (stepResult.status !== "passed") { + throw stepResult.error; + } + } catch (error) { + // Enrich ScenarioTimeoutError with current step information + if (error instanceof ScenarioTimeoutError) { + throw new ScenarioTimeoutError( + error.scenarioName, + error.timeoutMs, + error.elapsedMs, + { + cause: error, + currentStepName: step.name, + currentStepIndex: stepIndex, + }, + ); + } + throw error; } } } diff --git a/packages/probitas-runner/scenario_runner_test.ts b/packages/probitas-runner/scenario_runner_test.ts index 2475de7..855e0b4 100644 --- a/packages/probitas-runner/scenario_runner_test.ts +++ b/packages/probitas-runner/scenario_runner_test.ts @@ -64,7 +64,7 @@ Deno.test("ScenarioRunner run runs scenario with single step (fail)", async () = status: "failed", steps: [{ status: "failed", - error: new Error("ng"), + error: "ng", }], metadata: { name: "Test Scenario", @@ -139,7 +139,7 @@ Deno.test("ScenarioRunner run runs scenario with multiple steps (fail)", async ( }, { status: "failed", - error: new Error("2"), + error: "2", }, ], metadata: { diff --git a/packages/probitas-runner/step_runner.ts b/packages/probitas-runner/step_runner.ts index 804d3a7..a225beb 100644 --- a/packages/probitas-runner/step_runner.ts +++ b/packages/probitas-runner/step_runner.ts @@ -13,7 +13,7 @@ import { retry } from "./utils/retry.ts"; import { createStepContext } from "./context.ts"; import { mergeSignals } from "./utils/signal.ts"; import { Skip } from "./skip.ts"; -import { StepTimeoutError } from "./errors.ts"; +import { ScenarioTimeoutError, StepTimeoutError } from "./errors.ts"; const DEFAULT_STEP_TIMEOUT = 30000; const DEFAULT_STEP_RETRY_MAX_ATTEMPTS = 1; @@ -23,14 +23,6 @@ type ResourceStep = StepDefinition & { kind: "resource" }; type SetupStep = StepDefinition & { kind: "setup" }; type ExecutionStep = StepDefinition & { kind: "step" }; -/** - * Error with retry metadata attached - * @internal - */ -interface ErrorWithRetryMetadata extends Error { - __retryAttemptNumber?: number; -} - export class StepRunner { #reporter: Reporter; #scenarioMetadata: ScenarioMetadata; @@ -61,32 +53,92 @@ export class StepRunner { const timeout = this.#resolveTimeout(step); const retryConfig = this.#resolveRetry(step); - const signal = mergeSignals( - ctx.signal, - AbortSignal.timeout(timeout), - ); const result = await timeit(() => { return retry( - () => deadline(this.#run(ctx, step, stack), timeout, { signal }), + async (attempt) => { + // Create timeout signal for each retry attempt + using attemptTimeoutSignal = StepTimeoutError.timeoutSignal(timeout, { + stepName: step.name, + attemptNumber: attempt, + }); + const attemptSignal = mergeSignals(ctx.signal, attemptTimeoutSignal); + + // Create attempt-specific context with timeout signal + const attemptCtx = { ...ctx, signal: attemptSignal }; + + return await deadline( + this.#run(attemptCtx, step, stack), + timeout, + { signal: attemptSignal }, + ); + }, { ...retryConfig, - signal, + signal: ctx.signal, // For canceling retry delays only + shouldRetry: (error) => { + // Don't retry on timeout errors - they indicate the operation is too slow + // for the configured timeout, so retrying with the same timeout will fail again. + if ( + error instanceof StepTimeoutError || + error instanceof ScenarioTimeoutError + ) { + return false; + } + return true; + }, }, ); }); - // Enrich timeout errors with retry context and elapsed time + // Process timeout errors let error = result.status === "failed" ? result.error : undefined; - if (error && isTimeoutError(error)) { - const attemptNumber = - (error as ErrorWithRetryMetadata).__retryAttemptNumber ?? 1; - error = new StepTimeoutError( - step.name, - timeout, - attemptNumber, - result.duration, - { cause: error }, - ); + if (error) { + // Check for ScenarioTimeoutError first (from signal or thrown) + if ( + error instanceof ScenarioTimeoutError || + ctx.signal?.reason instanceof ScenarioTimeoutError + ) { + // Use the error with step info if available, otherwise enrich it + const scenarioError = error instanceof ScenarioTimeoutError + ? error + : ctx.signal!.reason as ScenarioTimeoutError; + + // If not already enriched with step info, add current step name + if (!scenarioError.currentStepName) { + error = new ScenarioTimeoutError( + scenarioError.scenarioName, + scenarioError.timeoutMs, + scenarioError.elapsedMs, + { + cause: scenarioError, + currentStepName: step.name, + }, + ); + } else { + // Already enriched, use as-is + error = scenarioError; + } + } else if (error instanceof StepTimeoutError) { + // Already enriched StepTimeoutError from signal + // Just update duration + error = new StepTimeoutError( + error.stepName, + error.timeoutMs, + error.attemptNumber, + result.duration, + { cause: error }, + ); + } else if ((error as Error).cause instanceof StepTimeoutError) { + // StepTimeoutError wrapped in another error (e.g., from signal abort) + const cause = (error as Error).cause as StepTimeoutError; + error = new StepTimeoutError( + cause.stepName, + cause.timeoutMs, + cause.attemptNumber, + result.duration, + { cause }, + ); + } } const stepResult: StepResult = result.status === "passed" @@ -202,7 +254,3 @@ function isDisposable(x: unknown): x is Disposable | AsyncDisposable { (Symbol.asyncDispose in x || Symbol.dispose in x) ); } - -function isTimeoutError(error: unknown): boolean { - return error instanceof DOMException && error.name === "TimeoutError"; -} diff --git a/packages/probitas-runner/step_runner_test.ts b/packages/probitas-runner/step_runner_test.ts index 18a2c55..b25518b 100644 --- a/packages/probitas-runner/step_runner_test.ts +++ b/packages/probitas-runner/step_runner_test.ts @@ -58,7 +58,7 @@ Deno.test("StepRunner run runs resource step (fail)", async () => { assertSpyCalls(step.fn as SpyLike, 1); expect(stepResult).toMatchObject({ status: "failed", - error: new Error("bar"), + error: "bar", metadata: { kind: "resource", name: "foo", @@ -110,7 +110,7 @@ Deno.test("StepRunner run runs setup step (fail)", async () => { assertSpyCalls(step.fn as SpyLike, 1); expect(stepResult).toMatchObject({ status: "failed", - error: new Error("bar"), + error: "bar", metadata: { kind: "setup", name: "foo", @@ -162,7 +162,7 @@ Deno.test("StepRunner run runs execution step (fail)", async () => { assertSpyCalls(step.fn as SpyLike, 1); expect(stepResult).toMatchObject({ status: "failed", - error: new Error("bar"), + error: "bar", metadata: { kind: "step", name: "foo", diff --git a/packages/probitas-runner/utils/retry.ts b/packages/probitas-runner/utils/retry.ts index e958cf6..d4e281b 100644 --- a/packages/probitas-runner/utils/retry.ts +++ b/packages/probitas-runner/utils/retry.ts @@ -9,14 +9,6 @@ import { delay } from "@std/async/delay"; -/** - * Error with retry metadata attached - * @internal - */ -interface ErrorWithRetryMetadata extends Error { - __retryAttemptNumber?: number; -} - /** * Configuration options for retry behavior */ @@ -34,6 +26,17 @@ export type RetryOptions = { /** AbortSignal to cancel retry delays */ signal?: AbortSignal; + + /** + * Predicate to determine if retry should continue after an error. + * Return false to stop retrying immediately. + * + * @param error - The raw error that occurred (not converted to Error) + * @param attempt - The 1-based attempt number that just failed + * @returns true to continue retrying, false to stop immediately + * @default Always returns true (retry all errors) + */ + shouldRetry?: (error: unknown, attempt: number) => boolean; }; /** @@ -43,7 +46,7 @@ export type RetryOptions = { * specified options. Delays between retries follow either linear or exponential backoff. * * @template T - Return type of the function - * @param fn - Function to execute (can be sync or async) + * @param fn - Function to execute (receives 1-based attempt number, can be sync or async) * @param options - Retry configuration options * @returns Promise resolving to the function's return value * @throws The last error encountered if all retry attempts fail @@ -54,7 +57,7 @@ export type RetryOptions = { * * const mockFetch = async () => ({ ok: true }); * const data = await retry( - * () => mockFetch(), + * (attempt) => mockFetch(), * { maxAttempts: 3, backoff: "exponential" } * ); * console.log(data); @@ -67,28 +70,42 @@ export type RetryOptions = { * const fetchData = async () => ({ status: "ok" }); * const controller = new AbortController(); * const data = await retry( - * () => fetchData(), + * (attempt) => fetchData(), * { maxAttempts: 5, backoff: "linear", signal: controller.signal } * ); * console.log(data); * ``` */ export async function retry( - fn: () => T | Promise, + fn: (attempt: number) => T | Promise, options: Readonly = {}, ): Promise { - const { maxAttempts = 1, backoff = "linear", signal } = options; - let lastError: Error | undefined; + const { + maxAttempts = 1, + backoff = "linear", + signal, + shouldRetry = () => true, + } = options; + let lastError: unknown; for (let attempt = 0; attempt < maxAttempts; attempt++) { + signal?.throwIfAborted(); + try { - return await fn(); + return await fn(attempt + 1); // Pass 1-based attempt number } catch (error) { - lastError = error instanceof Error ? error : new Error(String(error)); + lastError = error; - // Attach retry metadata to the error for context - // This helps StepTimeoutError include retry information - (lastError as ErrorWithRetryMetadata).__retryAttemptNumber = attempt + 1; + // Check if retry should continue based on raw error and attempt number + if (!shouldRetry(error, attempt + 1)) { + throw error; + } + + // Don't retry if the external signal (passed to retry) is aborted. + // This indicates the user cancelled the operation, so continuing is pointless. + if (signal?.aborted) { + throw error; + } if (attempt < maxAttempts - 1) { const t = backoff === "exponential" @@ -99,5 +116,6 @@ export async function retry( } } - throw lastError || new Error("Retry failed"); + // Throw the last error as-is (preserving its original type) + throw lastError; } diff --git a/packages/probitas-runner/utils/retry_test.ts b/packages/probitas-runner/utils/retry_test.ts index b3d303a..d4d0879 100644 --- a/packages/probitas-runner/utils/retry_test.ts +++ b/packages/probitas-runner/utils/retry_test.ts @@ -4,7 +4,7 @@ import { retry } from "./retry.ts"; Deno.test("retry configuration succeeds on first attempt", async () => { let attempts = 0; - const result = await retry(() => { + const result = await retry((_attempt) => { attempts++; return "success"; }); @@ -17,7 +17,7 @@ Deno.test("retry transient failures succeeds on second attempt", async () => { using time = new FakeTime(); let attempts = 0; - const promise = retry(() => { + const promise = retry((_attempt) => { attempts++; if (attempts < 2) { throw new Error("Temporary failure"); @@ -37,7 +37,7 @@ Deno.test("retry permanent failures throws error when all attempts fail", async using time = new FakeTime(); let attempts = 0; - const promise = retry(() => { + const promise = retry((_attempt) => { attempts++; throw new Error("Persistent failure"); }, { maxAttempts: 3 }); @@ -60,7 +60,7 @@ Deno.test("retry delays linear backoff timing", async () => { using time = new FakeTime(); let attempts = 0; - const promise = retry(() => { + const promise = retry((_attempt) => { attempts++; throw new Error("Always fails"); }, { maxAttempts: 3, backoff: "linear" }); @@ -87,7 +87,7 @@ Deno.test("retry delays exponential backoff timing", async () => { using time = new FakeTime(); let attempts = 0; - const promise = retry(() => { + const promise = retry((_attempt) => { attempts++; throw new Error("Always fails"); }, { maxAttempts: 4, backoff: "exponential" }); @@ -119,7 +119,7 @@ Deno.test("retry error handling can be aborted via signal", async () => { const controller = new AbortController(); let attempts = 0; - const promise = retry(() => { + const promise = retry((_attempt) => { attempts++; throw new Error("Always fails"); }, { maxAttempts: 5, backoff: "linear", signal: controller.signal }); @@ -139,3 +139,119 @@ Deno.test("retry error handling can be aborted via signal", async () => { // Should stop early due to abort (only 1 attempt completed) assertEquals(attempts, 1); }); + +Deno.test("retry shouldRetry predicate - stops on specific error", async () => { + let attempts = 0; + + class CustomError extends Error { + constructor(message: string) { + super(message); + this.name = "CustomError"; + } + } + + await assertRejects( + () => + retry( + (_attempt) => { + attempts++; + throw new CustomError("Should not retry"); + }, + { + maxAttempts: 3, + shouldRetry: (error) => !(error instanceof CustomError), + }, + ), + CustomError, + ); + + // Should only attempt once due to shouldRetry returning false + assertEquals(attempts, 1); +}); + +Deno.test("retry shouldRetry predicate - continues on other errors", async () => { + let attempts = 0; + + class CustomError extends Error { + constructor(message: string) { + super(message); + this.name = "CustomError"; + } + } + + await assertRejects( + () => + retry( + (_attempt) => { + attempts++; + throw new Error("Normal error"); + }, + { + maxAttempts: 3, + shouldRetry: (error) => !(error instanceof CustomError), + }, + ), + Error, + "Normal error", + ); + + // Should retry all 3 attempts for normal errors + assertEquals(attempts, 3); +}); + +Deno.test("retry shouldRetry predicate - receives attempt number", async () => { + const receivedAttempts: number[] = []; + + await assertRejects( + () => + retry( + (_attempt) => { + throw new Error("Always fails"); + }, + { + maxAttempts: 3, + shouldRetry: (_error, attempt) => { + receivedAttempts.push(attempt); + return true; + }, + }, + ), + Error, + ); + + // shouldRetry should be called with attempt 1, 2, 3 + assertEquals(receivedAttempts, [1, 2, 3]); +}); + +Deno.test("retry default behavior - retries all error types", async () => { + let attempts = 0; + + await assertRejects( + () => + retry((_attempt) => { + attempts++; + throw new Error("Normal error"); + }, { maxAttempts: 3 }), + Error, + "Normal error", + ); + + // Default shouldRetry returns true, so should retry all attempts + assertEquals(attempts, 3); +}); + +Deno.test("retry passes attempt number to callback", async () => { + const attemptNumbers: number[] = []; + + await assertRejects( + () => + retry((attempt) => { + attemptNumbers.push(attempt); + throw new Error("Fail"); + }, { maxAttempts: 3 }), + Error, + ); + + // Should receive 1, 2, 3 (1-based) + assertEquals(attemptNumbers, [1, 2, 3]); +});