Skip to content

feat(api): error-type predicates + fix rate-limit exit code - #122

Merged
joshdholtz merged 2 commits into
mainfrom
joshholtz/dx-969-api-error-predicates
Aug 20, 2026
Merged

feat(api): error-type predicates + fix rate-limit exit code#122
joshdholtz merged 2 commits into
mainfrom
joshholtz/dx-969-api-error-predicates

Conversation

@joshdholtz

@joshdholtz joshdholtz commented Aug 18, 2026

Copy link
Copy Markdown
Member

CLI error handling matched API error types by hardcoded string literals (apiErr.Type == "resource_missing", etc.), even though the v2 spec's error-type enum is already codegen'd into internal/api/types_gen.go as constants — they were just unused.

Adds predicates in internal/api backed by those constants:
IsAlreadyExists, IsNotFound, IsParameterError, IsRateLimited, IsUnauthorized, IsServerError. A future spec rename becomes a compile error instead of a silently-dropped match at each call site.

Bonus — fixes a real bug. ExitCodeFor only matched the synthesized rate_limit_exceeded fallback, not the spec's rate_limit_error that a real 429 actually carries — so real rate-limit errors returned exit 1 instead of 6. IsRateLimited accepts both, so it's fixed and locked by a matrix test case.

ExitCodeFor is swept onto the predicates now. The other literal sites (paywalls attach/detach from #111, and the parseError synthesized fallbacks) can move over as they land — the predicates already accept the legacy fallback strings, so nothing regresses in the meantime.

🤖 Generated with Claude Code


Note

Medium Risk
Changes CLI exit-code mapping for auth, not-found, and rate-limit errors (a script-facing contract). Behavior is expanded (403 and spec 429 now map correctly) rather than narrowed.

Overview
Adds IsAlreadyExists, IsNotFound, IsParameterError, IsRateLimited, IsUnauthorized, and IsServerError so callers classify APIErrors via OpenAPI-generated type constants instead of string literals. Spec renames then fail at compile time. Rate-limit and unauthorized predicates also accept the bodyless-4xx fallback strings from parseError.

ExitCodeFor now uses those predicates. That fixes a bug: real 429s carry rate_limit_error and previously fell through to exit 1 instead of 6. 403 authorization_error now maps to exit 4 as well. Wrapped errors are classified correctly.

Reviewed by Cursor Bugbot for commit 1bfa876. Bugbot is set up for automated code reviews on this repo. Configure here.

@linear-code

linear-code Bot commented Aug 18, 2026

Copy link
Copy Markdown
DX-969 Centralize API error-type matching behind api.Is…(err) predicates

Problem

CLI error handling matches API error types by hardcoded string literals scattered across the codebase — apiErr.Type == "resource_already_exists", == "parameter_error", == "resource_missing", etc. Each call site re-derives the errors.As + Status + Type dance (paywalls attach/detach, ExitCodeFor in internal/cli/runtime.go, the fallback in internal/api/errors.go, internal/api/projects.go). So:

  • a spec rename of an error type silently breaks matching — no compile error, the helpful hint just quietly stops firing
  • every new command copy-pastes the same boilerplate

We already generate the constants

The v2 OpenAPI spec defines the error type enum, and codegen already emits Go constants in internal/api/types_gen.go:

  • BadRequestTypeParameterError = "parameter_error"
  • ConflictTypeResourceAlreadyExists = "resource_already_exists"
  • NotFoundTypeResourceMissing = "resource_missing"
  • BadGatewayTypeServerError / InternalErrorTypeServerError = "server_error"

They're just unused — every site hardcodes the raw string instead.

Proposal

Add centralized predicate helpers in internal/api that wrap errors.As + the generated constant once:

func IsAlreadyExists(err error) bool
func IsNotFound(err error) bool
func IsParameterError(err error) bool
// + server_error / rate_limit / unauthorized as useful

Then sweep the existing string-literal sites onto them. Call sites become if api.IsAlreadyExists(err) { … }, and a future spec rename becomes a compile error (the constant changes) instead of a silently-dropped hint.

Scope

  • Add the predicates (backed by the generated constants) + a test per predicate that feeds a real fixture error and asserts the match.
  • Sweep known sites: internal/cli/paywalls.go (attach/detach), internal/cli/runtime.go (ExitCodeFor), internal/api/errors.go, internal/api/projects.go.

Origin

Surfaced reviewing the paywalls attach/detach PR (cli#111), which hardcodes "resource_already_exists" / "parameter_error".

Review in Linear

@joshdholtz

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread internal/api/error_predicates.go
@joshdholtz

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 83537cd. Configure here.

@joshdholtz
joshdholtz marked this pull request as ready for review August 19, 2026 03:21
Copilot AI lite review requested due to automatic review settings August 19, 2026 03:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@joshdholtz
joshdholtz requested review from a team and popcorn August 19, 2026 03:29
joshdholtz and others added 2 commits August 20, 2026 13:10
… code

Add IsAlreadyExists/IsNotFound/IsParameterError/IsRateLimited/IsUnauthorized/
IsServerError backed by the generated spec constants, so error classification
stops hardcoding type strings — a spec rename now breaks the build instead of
silently dropping a match.

ExitCodeFor matched only the synthesized rate_limit_exceeded fallback, not the
spec's rate_limit_error that a real 429 carries, so real rate-limit errors got
exit 1 instead of 6. Routed through IsRateLimited (accepts both), fixing it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The predicate claimed 401/403 but only matched authentication_error (401) and
the bodyless fallback, so a real 403 (authorization_error) fell through and
ExitCodeFor returned 1 instead of 4. Match the generated ForbiddenType constant.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@joshdholtz
joshdholtz force-pushed the joshholtz/dx-969-api-error-predicates branch from 83537cd to 1bfa876 Compare August 20, 2026 18:10
@joshdholtz

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1bfa876. Configure here.

@joshdholtz
joshdholtz merged commit a929438 into main Aug 20, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants