chore(cli): migrate @fern-api/ir-generator to use CliError#15004
chore(cli): migrate @fern-api/ir-generator to use CliError#15004
Conversation
FedeZara
changed the title
FedeZara
force-pushed
the
FedeZara/refactor/cli-error-ir-generator
branch
from
5ca410e to
789fb94
Compare
![hex-security-app[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
| if (package_.service != null) { | ||
| throw new Error("Found duplicate service for " + serviceId); | ||
| throw new CliError({ | ||
| message: "Found duplicate service for " + serviceId, |
There was a problem hiding this comment.
Before this PR, these throws were plain Error objects. resolveErrorCode() in CliError.ts resolves unknown errors to Unclassified, for which shouldReportToSentry() returns false. After this PR they are CliError with InternalError or ResolutionError codes, both of which have shouldReportToSentry() = true. The full error (including its .message) is then sent to Sentry via captureException in the catch block inside runInteractiveTask.
Error messages that now reach Sentry include user-supplied API definition identifiers: serviceId, webhookGroupId, websocketChannelId, subpackageId (this file), plus JSON.stringify(endpointDeclaration) in convertTransport.ts and type/wire-key names in several other migrated files. The SENTRY_DSN is baked into the production CLI binary at publish time (see .github/workflows/publish-cli.yml), so this affects all users who have not set FERN_DISABLE_TELEMETRY=true.
Concrete impact: proprietary API definition names and structure are sent to Fern's Sentry instance whenever these error paths are hit, which was not the case before this PR.
Prompt To Fix With AI
For any CliError whose message embeds user-supplied values (serviceId, webhookGroupId, subpackageId, JSON.stringify(endpointDeclaration), wireKey, typeId, etc.) that is assigned an InternalError or ResolutionError code (both of which are Sentry-reportable), either:
1. Strip the dynamic user data from the message and move it to a separate Sentry 'extra' context field that is only attached at the captureException call site, OR
2. Change the code to ValidationError / ConfigError / ReferenceError for those specific call sites where the error is triggered by user configuration rather than a true programming bug (those codes are not Sentry-reported), OR
3. Add a Sentry beforeSend hook in SentryClient that scrubs or truncates error messages before transmission.
Files affected: PackageTreeGenerator.ts (serviceId, webhookGroupId, websocketChannelId, subpackageId), convertTransport.ts (JSON.stringify(endpointDeclaration), source.proto, relativeFilePath), convertExampleType.ts (discriminant, rawValueType, typeBeingExemplified), DynamicSnippetsConverter.ts (endpoint.id, typeId), validateObjectExample.ts (wireKey), validateTypeReferenceExample.ts, and others migrated in this PR.Severity: low | Confidence: 78%
FedeZara
force-pushed
the
FedeZara/refactor/cli-error-ir-generator
branch
2 times, most recently
from
0ec78fb to
0704cdc
Compare
FedeZara
force-pushed
the
FedeZara/refactor/cli-error-ir-generator
branch
from
0704cdc to
4465a06
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| throw new CliError({ | ||
| message: "Found duplicate webhook group for " + webhookGroupId, | ||
| code: CliError.Code.InternalError | ||
| }); | ||
| } | ||
| package_.webhooks = webhookGroupId; | ||
| } | ||
|
|
||
| public addWebSocketChannel(websocketChannelId: WebSocketChannelId, fernFilepath: FernFilepath): void { | ||
| const package_ = this.getPackageForFernFilepath(fernFilepath); | ||
| if (package_.webhooks != null) { | ||
| throw new Error("Found duplicate webhook group for " + websocketChannelId); | ||
| throw new CliError({ | ||
| message: "Found duplicate webhook group for " + websocketChannelId, | ||
| code: CliError.Code.InternalError | ||
| }); |
There was a problem hiding this comment.
The addWebSocketChannel method checks package_.webhooks instead of package_.websocket for duplicates, and the error message incorrectly says "webhook group" instead of "websocket channel". This will fail to detect duplicate websocket channels and may incorrectly throw an error when a webhook group exists.
// Should be:
if (package_.websocket != null) {
throw new CliError({
message: "Found duplicate websocket channel for " + websocketChannelId,
code: CliError.Code.InternalError
});
}Note: This is a pre-existing bug in the original code (line 88-89), but the PR changes the error on lines 99-102 without fixing it.
| throw new CliError({ | |
| message: "Found duplicate webhook group for " + webhookGroupId, | |
| code: CliError.Code.InternalError | |
| }); | |
| } | |
| package_.webhooks = webhookGroupId; | |
| } | |
| public addWebSocketChannel(websocketChannelId: WebSocketChannelId, fernFilepath: FernFilepath): void { | |
| const package_ = this.getPackageForFernFilepath(fernFilepath); | |
| if (package_.webhooks != null) { | |
| throw new Error("Found duplicate webhook group for " + websocketChannelId); | |
| throw new CliError({ | |
| message: "Found duplicate webhook group for " + websocketChannelId, | |
| code: CliError.Code.InternalError | |
| }); | |
| throw new CliError({ | |
| message: "Found duplicate webhook group for " + webhookGroupId, | |
| code: CliError.Code.InternalError | |
| }); | |
| } | |
| package_.webhooks = webhookGroupId; | |
| } | |
| public addWebSocketChannel(websocketChannelId: WebSocketChannelId, fernFilepath: FernFilepath): void { | |
| const package_ = this.getPackageForFernFilepath(fernFilepath); | |
| if (package_.websocket != null) { | |
| throw new CliError({ | |
| message: "Found duplicate websocket channel for " + websocketChannelId, | |
| code: CliError.Code.InternalError | |
| }); | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Docs Generation Benchmark ResultsComparing PR branch against latest nightly baseline on
Docs generation runs |
2 participants
Description
Migrates
@fern-api/ir-generatorto use explicitCliErrorerror codes on everyfailAndThrow/failWithoutThrowingcall site.This is one of the package migration PRs that follow the error classification system introduced in #14749.
Changes Made
Assigned typed error codes across call sites in
packages/cli/generation/ir-generator/src/.Error codes used
IR_CONVERSION_ERRORVALIDATION_ERRORREFERENCE_ERRORPARSE_ERRORFiles touched (grouped by area)
ir-generator/src/covering type declaration resolution, extended properties, and conversion utilitiesgetTypeDeclaration.ts,addExtendedPropertiesToIr.tsTesting
pnpm test, excluding e2e and pre-existing environment failures)