Implement Phase 4 timeout functionality with full builder chain support#5
Merged
adelrodriguez merged 1 commit intomainfrom Feb 26, 2026
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughAdds timeout functionality to a try-retry library by introducing a TimeoutController interface for managing timeout state and promise racing, integrating timeout checks throughout the async runner execution paths, exporting TimeoutError and timeout utilities in the public API, and adding comprehensive test coverage for timeout scenarios across sync/async contexts. Updates type signatures to reflect timeout outcomes and marks the timeout phase as complete in the project plan. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Feb 23, 2026
Owner
Author
This was referenced Feb 23, 2026
adelrodriguez
changed the base branch from
02-22-split_run_into_sync_run_and_async_runasync_
to
graphite-base/5
Owner
Author
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Greptile SummaryThis PR implements Phase 4 timeout functionality with comprehensive integration across the execution pipeline. The implementation correctly enforces timeouts at all critical points - before attempts, during try/catch execution, and during retry delays. Key implementation highlights:
Minor findings:
The implementation is well-architected and handles both synchronous and asynchronous timeout enforcement correctly. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start([executeRunCore]) --> CreateTimeout[Create TimeoutController]
CreateTimeout --> BeforeAttempt{Check timeout<br/>before attempt}
BeforeAttempt -->|Exceeded| ReturnTimeout1[Return TimeoutError]
BeforeAttempt -->|OK| TryExec[Execute try function]
TryExec --> IsPromise{Returns<br/>Promise?}
IsPromise -->|Yes| RaceAsync[Race promise<br/>with timeout]
IsPromise -->|No| CheckSync{Check timeout<br/>after sync result}
RaceAsync --> AsyncResult{Timeout<br/>won race?}
AsyncResult -->|Yes| ReturnTimeout2[Return TimeoutError]
AsyncResult -->|No| Success1[Return result]
CheckSync -->|Exceeded| ReturnTimeout3[Return TimeoutError]
CheckSync -->|OK| Success2[Return result]
TryExec --> TryError[Try throws error]
TryError --> CheckTimeout1{Check timeout<br/>after error}
CheckTimeout1 -->|Exceeded| ReturnTimeout4[Return TimeoutError]
CheckTimeout1 -->|OK| CheckRetry{Should<br/>retry?}
CheckRetry -->|No| HasCatch{Has catch<br/>handler?}
CheckRetry -->|Yes| CheckTimeoutDelay{Check timeout<br/>before delay}
CheckTimeoutDelay -->|Exceeded| ReturnTimeout5[Return TimeoutError]
CheckTimeoutDelay -->|OK| RaceDelay[Race sleep<br/>with timeout]
RaceDelay --> DelayResult{Timeout<br/>won race?}
DelayResult -->|Yes| ReturnTimeout6[Return TimeoutError]
DelayResult -->|No| BeforeAttempt
HasCatch -->|No| ReturnUnhandled[Return UnhandledException]
HasCatch -->|Yes| ExecCatch[Execute catch handler]
ExecCatch --> IsCatchPromise{Catch returns<br/>Promise?}
IsCatchPromise -->|Yes| RaceCatch[Race catch promise<br/>with timeout]
IsCatchPromise -->|No| CheckCatchSync{Check timeout<br/>after catch}
RaceCatch --> CatchResult{Timeout<br/>won race?}
CatchResult -->|Yes| ReturnTimeout7[Return TimeoutError]
CatchResult -->|No| ReturnMapped1[Return mapped error]
CheckCatchSync -->|Exceeded| ReturnTimeout8[Return TimeoutError]
CheckCatchSync -->|OK| ReturnMapped2[Return mapped error]
Last reviewed commit: e215d1d |
Owner
Author
adelrodriguez
force-pushed
the
02-22-implement_phase_4_timeout_functionality_with_full_builder_chain_support
branch
from
d793553 to
e215d1d
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+17
to
+22
| const createTimeoutError = (cause?: unknown): TimeoutError => { | ||
| if (timeoutMs === undefined) { | ||
| return new TimeoutError({ cause }) | ||
| } | ||
|
|
||
| return new TimeoutError({ cause: cause ?? createTimeoutCause(timeoutMs) }) |
There was a problem hiding this comment.
Dead code - the if (timeoutMs === undefined) branch is unreachable. Both checkDidTimeout and raceWithTimeout only call createTimeoutError when timeoutMs !== undefined
Suggested change
| const createTimeoutError = (cause?: unknown): TimeoutError => { | |
| if (timeoutMs === undefined) { | |
| return new TimeoutError({ cause }) | |
| } | |
| return new TimeoutError({ cause: cause ?? createTimeoutCause(timeoutMs) }) | |
| const createTimeoutError = (cause?: unknown): TimeoutError => { | |
| return new TimeoutError({ cause: cause ?? createTimeoutCause(timeoutMs) }) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/timeout.ts
Line: 17-22
Comment:
Dead code - the `if (timeoutMs === undefined)` branch is unreachable. Both `checkDidTimeout` and `raceWithTimeout` only call `createTimeoutError` when `timeoutMs !== undefined`
```suggestion
const createTimeoutError = (cause?: unknown): TimeoutError => {
return new TimeoutError({ cause: cause ?? createTimeoutCause(timeoutMs) })
}
```
How can I resolve this? If you propose a fix, please make it concise.
adelrodriguez
deleted the
02-22-implement_phase_4_timeout_functionality_with_full_builder_chain_support
branch
This was referenced Feb 27, 2026
Merged
This was referenced Mar 6, 2026
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
1 participant
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.
This pull request implements timeout functionality for the hardtry library, completing Phase 4 of the development plan.
Key Changes:
TimeoutControllerinterface andcreateTimeoutControllerfunction to manage timeout logic, including deadline checking and racing promises against timeoutsTimeoutErrorinstances as control errors that bypass normal catch mappingThe implementation uses
Promise.race()to enforce timeouts on async operations and tracks elapsed time for synchronous timeout checks. All timeout violations returnTimeoutErrorinstances that are treated as control errors and propagate through the execution chain without transformation.Greptile Summary
Implements Phase 4 timeout functionality with comprehensive integration into the execution flow.
Key additions:
TimeoutControllerinterface manages deadline tracking usingDate.now()and races promises against timeouts with proper cleanupPromise.race), before retry delays, during retry delays, and in both sync/async catch handlersTimeoutErrortreated as control error that bypasses catch mapping and propagates unchangedImplementation quality:
Confidence Score: 5/5
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Start[Start Execution] --> CreateController[Create TimeoutController] CreateController --> CheckTimeout1{Check Timeout<br/>Before Attempt?} CheckTimeout1 -->|Expired| ReturnTimeout1[Return TimeoutError] CheckTimeout1 -->|OK| ExecuteTry[Execute Try Function] ExecuteTry --> IsAsync{Is Promise?} IsAsync -->|Yes| RacePromise[Race Promise<br/>vs Timeout] RacePromise --> RaceResult{Result?} RaceResult -->|Timeout| ReturnTimeout2[Return TimeoutError] RaceResult -->|Success| ReturnValue[Return Value] RaceResult -->|Error| HandleError[Handle Error] IsAsync -->|No| CheckTimeout2{Check Timeout<br/>After Sync?} CheckTimeout2 -->|Expired| ReturnTimeout3[Return TimeoutError] CheckTimeout2 -->|OK| ReturnValue HandleError --> ShouldRetry{Should<br/>Retry?} ShouldRetry -->|No| ExecuteCatch[Execute Catch Handler] ShouldRetry -->|Yes| CheckTimeout3{Check Timeout<br/>Before Delay?} CheckTimeout3 -->|Expired| ReturnTimeout4[Return TimeoutError] CheckTimeout3 -->|OK| RaceDelay[Race Sleep Delay<br/>vs Timeout] RaceDelay --> DelayResult{Result?} DelayResult -->|Timeout| ReturnTimeout5[Return TimeoutError] DelayResult -->|Complete| CheckTimeout1 ExecuteCatch --> CatchAsync{Is Promise?} CatchAsync -->|Yes| RaceCatch[Race Catch<br/>vs Timeout] RaceCatch --> CatchResult{Result?} CatchResult -->|Timeout| ReturnTimeout6[Return TimeoutError] CatchResult -->|Success| ReturnMapped[Return Mapped Value] CatchAsync -->|No| CheckTimeout4{Check Timeout<br/>After Catch?} CheckTimeout4 -->|Expired| ReturnTimeout7[Return TimeoutError] CheckTimeout4 -->|OK| ReturnMappedLast reviewed commit: d793553