Change driveGen to throw errors instead of returning UnhandledException#30
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
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. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR refactors error handling in generator execution logic. The changes simplify error propagation by removing UnhandledException wrapping behavior and eliminating ControlError handling. Type assertions in tests are updated to include UnhandledException in the possible return types. Test files are rewritten to use try/catch blocks instead of expecting wrapped exceptions, verifying direct error propagation for yielded promises, final promises, and factory/generator errors. A wrapper test parameter is adjusted to ignore execution context. 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 |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/gen.ts`:
- Line 25: executeAsyncGenerator currently awaits promises (e.g., the assignment
to currentValue from initialValue and the awaits around lines 45–46) which lets
rejected promises short-circuit the async generator and skip generator-local
catch/finally blocks; change those awaits to try/catch blocks that on rejection
call iterator.throw(error) to re-inject the error into the generator before
rethrowing (preserve returned/rejected behavior), so generator cleanup runs;
update executeAsyncGenerator's logic where it awaits initialValue/currentValue
and the later awaits to wrap awaits with try { currentValue = await ... } catch
(err) { await safeIteratorThrow(iterator, err); throw err; } and add a
regression test like try/finally { yield* use(Promise.reject(...)) } to ensure
finally runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4778ec8-e56a-4588-924b-239100877c70
📒 Files selected for processing (4)
src/__tests__/types.test.tssrc/lib/__tests__/gen.test.tssrc/lib/executors/__tests__/base.test.tssrc/lib/gen.ts
adelrodriguez
force-pushed
the
03-07-change_drivegen_to_throw_errors_instead_of_returning_unhandledexception_
branch
from
56c3a30 to
28f232d
Compare
adelrodriguez
deleted the
03-07-change_drivegen_to_throw_errors_instead_of_returning_unhandledexception_
branch
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28f232dc46
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (error) { | ||
| if (checkIsControlError(error)) { | ||
| return error as GenErrors<TYield> | ||
| } | ||
|
|
||
| return new UnhandledException(undefined, { cause: error }) as GenErrors<TYield> | ||
| currentStep = iterator.throw(error) | ||
| continue |
There was a problem hiding this comment.
Rejected yielded promises are now always fed back with iterator.throw(error), including TimeoutError/CancellationError/Panic. In this path, a generator try/catch (or finally with return) can swallow those control errors and return a success value, so cancellation/timeout can be silently ignored instead of propagating. Before this commit, these control errors were returned immediately and could not be intercepted by generator code, which preserved fail-fast control semantics.
Useful? React with 👍 / 👎.
1 participant
This pull request changes the error handling behavior in the generator execution system from wrapping exceptions in
UnhandledExceptionto throwing the original errors directly.Key Changes:
Generator execution now throws original errors: The
driveGenfunction no longer catches and wraps exceptions inUnhandledException. Instead, it allows the original errors to propagate naturally.Preserved control error handling: Special error types like
TimeoutError,CancellationError, andPaniccontinue to be preserved and thrown as-is, maintaining their specific error semantics.Simplified async error propagation: Rejected promises in the async execution path now reject with their original reasons rather than being wrapped in
UnhandledException.Updated type assertions: Test type assertions now include
UnhandledExceptionin the expected union types to reflect that it can still be returned in certain error scenarios.Comprehensive test updates: All tests have been updated to expect thrown errors using try-catch blocks instead of checking return values, including tests for factory errors, generator body errors, promise rejections, and non-Error thrown values.
The change makes error handling more predictable by preserving the original error types and stack traces while maintaining backward compatibility for control flow errors.