fix: Catch exceptions from user-provided retry handlers to prevent orchestration crash#193
Merged
YunchuWang merged 2 commits intomainfrom Mar 24, 2026
Conversation
…chestration crashes When a user-provided retry handler (AsyncRetryHandler) or handleFailure predicate (RetryPolicy) throws an exception, the error previously propagated uncaught through tryHandleRetry() -> handleFailedTask() -> processEvent() -> execute(), crashing the entire orchestration. The fix wraps these user-provided function calls in try-catch blocks within tryHandleRetry(). When an exception is caught: - A warning is logged (EVENT_RETRY_HANDLER_EXCEPTION = 737) - The method returns false (don't retry) - The task fails normally with its original error - The orchestrator can catch the TaskFailedError as usual Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents user-provided retry callbacks (custom AsyncRetryHandler and RetryPolicy.handleFailure) from crashing the entire orchestration when they throw, by safely catching those exceptions in the orchestration executor and allowing the original task failure to flow normally.
Changes:
- Add
try/catcharound retry policy delay computation and custom retry handler evaluation intryHandleRetry(). - Emit a new warning log event when retry evaluation throws, and treat it as “don’t retry”.
- Add regression tests covering both retry-handler and
handleFailure-predicate exception scenarios (including orchestrator catch behavior).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/durabletask-js/src/worker/orchestration-executor.ts | Catches exceptions from user retry evaluation logic to avoid failing the whole orchestration with the handler error. |
| packages/durabletask-js/src/worker/logs.ts | Introduces a dedicated log event ID and helper for retry-evaluation exceptions. |
| packages/durabletask-js/test/orchestration_executor.spec.ts | Adds tests ensuring retry callback exceptions don’t bypass normal task-failure semantics. |
kaibocai
approved these changes
Mar 24, 2026
…ctured properties
kaibocai
approved these changes
Mar 24, 2026
YunchuWang
deleted the
copilot-finds/bug/retry-handler-exception-crashes-orchestration
branch
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #192
Problem
When a user-provided retry handler (AsyncRetryHandler) or handleFailure predicate (RetryPolicy) throws an exception, the error propagates uncaught through the orchestration executor, crashing the entire orchestration instead of just failing the individual task.
Changes