-
-
Notifications
You must be signed in to change notification settings - Fork 0
Improve timeout error handling and maxFailures behavior #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
| }, | ||
| }); | ||
| } | ||
|
Comment on lines
+128
to
157
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same cleanup pattern is duplicated in both StepTimeoutError.timeoutSignal and ScenarioTimeoutError.timeoutSignal. Consider extracting this into a shared helper function to reduce code duplication and improve maintainability.