Skip to content

Conversation

@lambdalisue
Copy link
Member

Summary

  • Fix scenario timeout to throw ScenarioTimeoutError instead of AbortError
  • Add timeout option to scenario configuration for per-scenario timeout control
  • Simplify timeout handling logic by removing deadline() dependency

Why

Problem: When a scenario timed out, the runner was throwing a generic AbortError
instead of the documented ScenarioTimeoutError. This happened because deadline()
threw DeadlineError which wasn't detected by the timeout error check.

Solution: Remove deadline() and rely solely on AbortSignal.timeout(), which
directly throws the expected TimeoutError (DOMException). The timeout detection now
also recognizes StepTimeoutError and properly converts it to ScenarioTimeoutError.

Enhancement: Users can now set timeouts per scenario using the scenario()
options, allowing fine-grained control over execution time limits. This is useful
for scenarios that need different timeout constraints than the global default.

Priority: scenario.timeout > RunOptions.timeout

Test Plan

  • All existing tests pass
  • Scenario timeout tests verify ScenarioTimeoutError is thrown correctly
  • Scenario-level timeout takes precedence over global timeout
  • Backward compatibility maintained (no breaking changes)

Previously, when a scenario timeout occurred, AbortError was thrown instead
of the documented ScenarioTimeoutError. This was caused by using deadline()
which throws DeadlineError that wasn't detected by isTimeoutError().

This fix:
- Removes deadline() usage, relying solely on AbortSignal.timeout()
- Detects StepTimeoutError and converts it to ScenarioTimeoutError
- Simplifies the timeout handling logic

Fixes issue where scenario timeout was reported as AbortError instead of
the expected ScenarioTimeoutError with proper context.
…-level timeout configuration

Users can now configure timeout per scenario using the scenario() options:

  scenario("Quick Test", { timeout: 5000 })
    .step("Call API", async () => { ... })
    .build();

This timeout applies to the total execution time of all steps in the
scenario. When the scenario exceeds this limit, ScenarioTimeoutError is
thrown.

Priority: scenario.timeout > RunOptions.timeout

Changes:
- Add timeout?: number field to ScenarioOptions, ScenarioDefinition, and
  ScenarioMetadata in @probitas/core
- Update ScenarioBuilder to include timeout in built definition
- Update Runner to respect scenario-level timeout over global RunOptions
  timeout

This allows fine-grained timeout control for specific scenarios while
maintaining backward compatibility with global timeouts.
Copilot AI review requested due to automatic review settings January 8, 2026 06:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes scenario timeout error handling and adds per-scenario timeout configuration. The main change removes the deadline() dependency in favor of AbortSignal.timeout(), which directly throws the expected TimeoutError (DOMException). Additionally, scenarios can now specify their own timeout values that override the global RunOptions timeout.

Key changes:

  • Simplified timeout handling by removing deadline() and relying solely on AbortSignal.timeout()
  • Added timeout field to ScenarioOptions, ScenarioDefinition, and ScenarioMetadata with comprehensive documentation
  • Enhanced timeout error detection to recognize both TimeoutError (DOMException) and StepTimeoutError, properly converting them to ScenarioTimeoutError

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/probitas-runner/runner.ts Removed deadline() dependency, simplified timeout handling logic, added scenario-level timeout precedence (scenario.timeout ?? timeout), and enhanced isTimeoutError() to detect StepTimeoutError
packages/probitas-core/types/scenario.ts Added timeout field to ScenarioOptions, ScenarioDefinition, and ScenarioMetadata interfaces with comprehensive JSDoc documentation
packages/probitas-builder/scenario_builder.ts Updated build() method to include timeout field from scenario options in the generated definition

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +176 to +182
// Priority: scenario timeout > RunOptions timeout
const effectiveTimeout = scenario.timeout ?? timeout;
const scenarioResult = effectiveTimeout > 0
? await this.#runWithTimeout(
scenarioRunner,
scenario,
timeout,
effectiveTimeout,
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new scenario-level timeout feature lacks test coverage. While there are existing tests for RunOptions timeout, there are no tests verifying that scenario.timeout takes precedence over RunOptions.timeout, or that scenarios with different timeout values work correctly together.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/probitas-runner/runner.ts 79.31% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@lambdalisue lambdalisue merged commit cc6284f into main Jan 8, 2026
9 checks passed
@lambdalisue lambdalisue deleted the fix/timeout branch January 8, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants