feat: add throwOnError option to generate function#3362
Conversation
Added `throwOnError` option to `GlobalOptions` that, when set to true, causes the `generate` function to throw errors instead of just logging them. This allows programmatic consumers to handle errors with their own error handling logic. Closes orval-labs#2290
📝 WalkthroughWalkthroughThis PR adds a new ChangesConditional error re-throwing in generate
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new throwOnError option that, when enabled, causes generate to rethrow errors from generateSpec instead of only logging them.
Changes:
- Add
throwOnError?: booleantoGlobalOptions. - Rethrow caught errors in three
generateSpeccall sites whenthrowOnErroris set.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/types.ts | Adds the new throwOnError field to GlobalOptions. |
| packages/orval/src/generate.ts | Rethrows errors from generateSpec in three locations when throwOnError is true. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (options?.throwOnError) { | ||
| throw error; | ||
| } | ||
| logError(error, projectName); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/orval/src/generate.ts (1)
59-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply
throwOnErrorin the config-file watch callback too.Line 59-61 still swallows generation errors in watch mode for config-file flows, so behavior is inconsistent with the new
throwOnErrorcontract.Suggested fix
await startWatcher( options.watch, async () => { resetWarnings(); try { await generateSpec(workspace, normalizedOptions, projectName); } catch (error) { + if (options?.throwOnError) { + throw error; + } logError(error, projectName); } if (options.failOnWarnings && getWarningCount() > 0) { throw new Error( `Process failed with ${getWarningCount()} warning(s) due to failOnWarnings option`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/orval/src/generate.ts` around lines 59 - 61, In the catch block inside the config-file watch callback (currently calling logError(error, projectName)), respect the new throwOnError contract by checking the watcher/config option and rethrowing the caught error when enabled: call logError(error, projectName) as today, then if the config or options.loaded.throwOnError (or the existing throwOnError variable) is truthy, rethrow error so the watch flow doesn't swallow it; update the catch to reference the same throwOnError flag used elsewhere so behavior is consistent across generation paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/orval/src/generate.ts`:
- Around line 59-61: In the catch block inside the config-file watch callback
(currently calling logError(error, projectName)), respect the new throwOnError
contract by checking the watcher/config option and rethrowing the caught error
when enabled: call logError(error, projectName) as today, then if the config or
options.loaded.throwOnError (or the existing throwOnError variable) is truthy,
rethrow error so the watch flow doesn't swallow it; update the catch to
reference the same throwOnError flag used elsewhere so behavior is consistent
across generation paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ea00515-cb79-4f76-aa08-8cd9366795c0
📒 Files selected for processing (2)
packages/core/src/types.tspackages/orval/src/generate.ts
melloware
left a comment
melloware
left a comment
There was a problem hiding this comment.
Build is failing and documentation needed based on your other ticket
ded-furby
left a comment
ded-furby
left a comment
There was a problem hiding this comment.
One behavior worth documenting in this PR: when generate() loads a config file with multiple named projects, throwOnError now fails fast on the first generateSpec error and exits the for...of loop before later projects run. That is probably the right tradeoff for programmatic callers, but it is a semantic change from the current "log and continue" behavior, not just a different error surface.
I’d call that out explicitly in docs/content/docs/reference/integration.mdx alongside the new option example so consumers know enabling throwOnError changes multi-project runs from best-effort to fail-fast.
|
@ded-furby any chance you want to resubmit this PR with the documentation change? It seems the OP has let this branch go dead... |
Summary
Added
throwOnErroroption to thegeneratefunction that allows callers to opt into error-throwing behavior.Why
When using orval programmatically, errors are caught internally and only logged, making it difficult for callers to implement custom error handling.
What changed
throwOnError?: booleantoGlobalOptionstry/catchblocks ingenerate.tsto re-throw when enabledCloses #2290
Summary by CodeRabbit
Release Notes
throwOnErrorconfiguration setting to control error handling behavior during generation operations. When enabled, errors are re-thrown for improved error visibility and control.