feat(errors): tighten error taxonomy + surface code/requestId#13
Merged
feat(errors): tighten error taxonomy + surface code/requestId#13
Conversation
- Replace ad-hoc AuthError/ValidationError/NotFoundError/ApiRequestError
with a P4-aligned hierarchy:
DevhelmError (umbrella, exit 1)
├── DevhelmValidationError (exit 4)
├── DevhelmApiError (exit 11) — status, code, requestId
│ ├── DevhelmAuthError (401/403)
│ └── DevhelmNotFoundError (404)
└── DevhelmTransportError (exit 12)
- Pin EXIT_CODES contract: VALIDATION=4, CHANGES_PENDING=10, API=11,
TRANSPORT=12, PARTIAL_FAILURE=13
- checkedFetch maps HTTP status → typed subclass and extracts code +
requestId (header wins over body) onto every thrown error
- response-validation throws DevhelmValidationError (not bare ZodError)
so exit-code routing is uniform regardless of failure side
- Regenerated schemas + descriptions pull through ErrorResponse with
code+requestId
Made-with: Cursor
Three related YAML-apply fixes uncovered while auditing the codegen pipelines for stray typecasts and silent drift: 1. Status page reorder was sending `groupId: null` for every component in the batch. The API treats the reorder payload as a full upsert (sets groupId to whatever's provided), so any deploy that triggered a reorder silently moved every grouped component back to "ungrouped". Fix: extend `applyReorder` to receive the desired YAML alongside the ordered IDs, and resolve each component's groupId from its YAML `group` reference. Also covered by a new BDD assertion in `mono/tests/surfaces/cli/bdd/test_b_org_structure.py`. 2. `checkedFetch` returned `unknown`, which forced every YAML handler to launder the response through a `castEnvelope<T>(...)` typecast. That defeats the spec-driven types from `openapi-fetch` and would hide spec drift at runtime. Make `checkedFetch<TData>` generic so the response keeps its inferred shape, drop `castEnvelope`, and route ID extraction through a small `idToString` helper. 3. Zod validation errors used dotted paths (`monitors.0.config.url`) while the handwritten validator used bracket notation (`monitors[0].config.url`), so users saw two different shapes for the same conceptual field depending on which validator caught the issue. Add a `formatZodPath` helper that always renders array indices as `[N]` and matches the validator's style, with unit tests. Made-with: Cursor
…mands
- Replace ad-hoc exit codes (1, 2) with canonical EXIT_CODES taxonomy:
- VALIDATION (4) for local config/state/parse failures
- API (11) for auth/token failures and lock conflicts
- GENERAL (1) only for unclassified runtime/IO errors
- Remove `as {data?: ...}` typecasts after checkedFetch — rely on the
generic return type from the openapi-fetch client.
- Branch on DevhelmApiError.status (instead of substring-matching the
message) for 404 ("no lock") and 409 ("lock held") in deploy/* so
errors with those words in the body don't get misclassified.
- validate.ts now uses EXIT_CODES.VALIDATION for both schema-parse and
semantic errors, matching the unified DevhelmValidationError taxonomy.
Touches: validate, plan, init, import, deploy/{index,force-unlock},
auth/{login,token,context/use,context/delete}, state/{rm,show,pull},
monitors/{pause,resume}, incidents/resolve, dependencies/track,
alert-channels/test, webhooks/test.
Made-with: Cursor
…teEnvironmentRequest Picks up the mono spec change (`CreateEnvironmentRequest.isDefault` is now nullable; server defaults to false). Regenerates the OpenAPI types, Zod schemas, and flag descriptions accordingly: - api.generated.ts: `isDefault?: boolean | null` - api-zod.generated.ts: `z.boolean().nullish()` - descriptions.generated.ts: refreshed flag help text No imperative or YAML command logic changed — the field simply becomes optional on `devhelm environments create` and in the YAML loader. Made-with: Cursor
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.
Summary
Replaces the ad-hoc
AuthError/ValidationError/NotFoundError/ApiRequestErrorset with a P4-aligned hierarchy:```
DevhelmError (umbrella, exit 1)
├── DevhelmValidationError (exit 4)
├── DevhelmApiError (exit 11) — status, code, requestId
│ ├── DevhelmAuthError (401/403)
│ └── DevhelmNotFoundError (404)
└── DevhelmTransportError (exit 12)
```
EXIT_CODES:VALIDATION=4,CHANGES_PENDING=10,API=11,TRANSPORT=12,PARTIAL_FAILURE=13.checkedFetchmaps HTTP status → typed subclass and extractscode+requestId(header wins over body) onto every thrown error.response-validationthrowsDevhelmValidationError(not bareZodError) so exit-code routing is uniform regardless of which side fails.Test plan
pnpm test(856 passed)tests/surfaces/cli/,tests/surfaces/evolution/surfaces/test_cli.py) greenMade with Cursor