feat(core): add StackError root with instance-level code - #154
Merged
Conversation
All nine Stack-domain error classes extended Error directly, so "is this a domain error or a bug?" — the first question a server's error middleware asks — could only be answered by exhausting a nine-arm instanceof ladder. serializeError() was exactly that ladder, and it carried an ordering hazard: StackVersionConflictError had to be tested before StackConflictError or 412 would silently serialize as 409. Add an abstract StackError root with an abstract instance `code`, and re-parent the nine leaves. Each leaf derives its instance code from its own static, so the literal is still written once per class and the static the spec documents keeps working. Membership in the hierarchy now carries a guarantee: a StackError always has a code, and every code has a status. IdGenerationError and InvalidDidError have no wire mapping and stay outside for that reason. No hierarchy beyond the root — StackVersionConflictError remains a sibling of StackConflictError, and the 409/412 split is unchanged. The code vocabulary moves to core as StackErrorCode, with wire-types keeping its public WireErrorCode name as an alias. core has no dependencies and cannot import from wire-types, and the union was already duplicated there: every literal existed as a static `code` on a core class. This revises the open question in #53, which guessed wire-types was the right home. serializeError() collapses onto err.code: the ladder becomes a status lookup plus an instanceof for the three classes carrying structured payload. Those three are leaves with no subtype relation, so the ordering trap is gone for good. Output is byte-identical, key order included. wire-types had no test setup and serializeError was covered only indirectly, through adapter-api's conformance tests. Add vitest to the package and pin the round trip directly, one case per taxonomy member. Closes #145 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014iKbdWJzDFtBzp1Jg7BTS1
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
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
Closes #145.
All nine Stack-domain error classes extended
Errordirectly, so "is this a domain error or a bug?" — the first question a server's error middleware asks — could only be answered by exhausting a nine-arminstanceofladder.serializeError()was exactly that ladder, and it carried an ordering hazard:StackVersionConflictErrorhad to be tested beforeStackConflictErroror a 412 would silently serialize as 409.StackErrorroot. Abstract,extends Error, with an abstract instancecode. The nine leaves are re-parented; each derives its instance code from its own static (override readonly code = StackValidationError.code), so the literal is still written once per class and the static the spec documents keeps working. Membership now carries a guarantee: aStackErroralways has acode, and every code has a status — which is what letsserializeErrordo a lookup instead of a chain of tests.StackVersionConflictErrorstays a sibling ofStackConflictError; the 409/412 split is untouched.IdGenerationErrorandInvalidDidErrorhave no wire mapping and deliberately stay outside.StackErrorCodemoves to core, withwire-typeskeeping its publicWireErrorCodename as an alias.corehas no dependencies and can't import fromwire-types, and the union was already duplicated there — every literal existed as a staticcodeon a core class. This revises the open question in APIAdapter collapses the error taxonomy: Stack* errors never survive the wire round trip #53, which guessedwire-typeswas the right home.serializeError()collapses ontoerr.code: a status lookup plus aninstanceoffor the three classes carrying structured payload (details,versionConflict,schemaDrift). Those three are leaves with no subtype relation, so the ordering trap is gone for good. Output is byte-identical, key order included.Spec
Yes — error mapping.
docs/spec/wire-format.md:instanceof StackErrorguarantees, whyIdGenerationError/InvalidDidErrorare excluded, and that the root adds no other structure.codeis now exposed per-instance as well as per-class, and the vocabulary's home is named. Also states the honest limitation that an instancecodediscriminates but doesn't narrow — TypeScript won't refine aStackErrorto a subclass from a literal check, so reaching payload fields still means aninstanceof.No wire shape changed, so
@haverstack/conformance-fixturesneeded no update — the existing fixtures pass unmodified, which is itself part of the evidence that serialization output is unchanged.Verification
All five, from a clean install:
New tests:
packages/core/tests/stack.test.ts§ error taxonomy — all nine descend fromStackError; instancecodeequals the static for each; codes are distinct;StackVersionConflictErroris not aStackConflictError(and vice versa);StackSchemaDriftErroris not either;IdGenerationError/InvalidDidError/plainErrorare outside; and errors thrown by realStackoperations are catchable asStackError.packages/wire-types/tests/errors.test.ts— new test setup for the package (vitest config + script;serializeErrorpreviously had no direct coverage, only indirect exercise via adapter-api's conformance tests). Pins code/status/message for every taxonomy member,nullfor non-StackErrorinput, 409-vs-412 separation, payload fields present only for the codes that define them, full serialize→deserialize class round trip, anderrorForStatusbehavior including the bodyless-409 degradation and the refusal to inferStackMigrationErrorfrom a bare 500.Checked by hand:
extends Errorfails 1 core test and 3 wire-types tests; restored and re-ran green. The tests pin the invariant, not the current text.dist/stack.js:super()→code = '...'→this.name = '...'. The base deliberately has no constructor and never readsthis.code, since subclass field initializers run aftersuper()returns.Notes for reviewers
get code() { return this.constructor.code }would avoid the per-class line, but TypeScript can't enforce abstract statics, so a future subclass that forgot its static would silently returnundefined. The abstract instance member is enforced at compile time.this.name = new.target.namein the base to delete ninethis.nameassignments. It makes a user-visible value depend on whether a bundler mangles class names.nameisn't part of the wire contract, but the risk buys nothing.IdGenerationOverflowError extends IdGenerationError(packages/core/src/id.ts:40) is an existing hierarchy outside the taxonomy and unaffected here — but neitherIdGenerationErrornorInvalidDidErroris exported from core's index, so the wire-types test can't reference them (the core test imports them by relative path). If they're meant to be catchable by apps, that's a separate export gap.Generated by Claude Code