Skip to content

Conversation

@bokelley
Copy link
Contributor

Summary

Fixed timeout handling tests in error-scenarios.test.js to correctly expect error results (success: false) instead of thrown exceptions.

Background

The tests were expecting TaskExecutor to throw TaskTimeoutError exceptions, but the implementation actually catches all exceptions internally and returns standardized error results via the TaskResult interface.

Why CI was passing: These tests are marked as "slow tests" and skipped in CI via skip: process.env.CI (line 17 of the test file). The failures only occurred in local development.

Changes

Updated 4 timeout-related tests:

  • ✅ "should timeout on working status after configured limit"
  • ✅ "should handle polling timeout during working status"
  • ✅ "should handle webhook timeout in submitted tasks" - Fixed async activity issue
  • ✅ "should handle zero and negative timeout values"

Test Results

All timeout tests now pass in local development:

// Before (BROKEN):
await assert.rejects(executor.executeTask(...), TaskTimeoutError);

// After (CORRECT):
const result = await executor.executeTask(...);
assert.strictEqual(result.success, false);
assert(result.error.includes('timed out'));

Architecture Verification

✅ Confirmed this is the correct protocol design:

  • TaskExecutor.ts line 489: Throws TaskTimeoutError internally
  • TaskExecutor.ts line 215: Catches all errors in public API
  • TaskExecutor.ts line 766-788: Converts to TaskResult with success: false
  • Follows Result pattern (not exception-based error handling)

Related Issues

Changeset

  • 📦 Patch version bump (test-only change)

🤖 Generated with Claude Code

Fixed timeout handling tests in error-scenarios.test.js to correctly expect
error results (success: false) instead of thrown exceptions.

Changes:
- Updated "should timeout on working status after configured limit" test
- Updated "should handle polling timeout during working status" test
- Fixed "should handle webhook timeout in submitted tasks" to properly await
  polling completion (was causing async activity after test ended)
- Updated "should handle zero and negative timeout values" test

Note: These tests are skipped in CI (marked as slow tests) which is why
CI was passing. The fixes ensure they pass in local development.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@bokelley bokelley merged commit 470151b into main Oct 28, 2025
7 checks passed
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