feat(openapi3): add per-type validation errors with cluster wrappers#1166
Conversation
…ndings
Today the document validation walker (T.Validate, Info.Validate, Paths.Validate, etc.)
returns plain errors.New(...) values for ~50 distinct failure modes, so
downstream code that wants to react to a specific validation failure has
to compare on the human-readable message string. That's brittle: any
upstream rewording silently breaks the consumer.
Introduce a typed ValidationError carrying a stable Code (kebab-case
subject-action shape, e.g. "info-version-required") in addition to the
existing Message. Callers can switch on the code via errors.As, while
existing string-matching code continues to work because Error() returns
the same message string.
This commit only converts the two error sites in Info.Validate to
demonstrate the migration pattern. Bulk conversion of remaining sites
(license, link, header, paths, operation, response, request_body,
parameter, server, schema, etc. — ~50 sites total) is intentionally
deferred so the API design conversation can settle first.
Backward compatibility: every site converts errors.New(msg) to
&ValidationError{Code: "...", Message: msg} keeping msg byte-identical.
All existing tests still pass.
|
Hey there! Agreed: errors should be easier to differentiate in a more Go way + strings are not reliable. I welcome this change however I think it'd be best to introduce types instead of variants of a code string. This way errors can be differentiated not on their string but on their type + they can be wrapped in multiple types + most error strings could be turned to consts. |
|
Thanks for the quick look. Agreed on the principle. 1. Granularity Strict one-type-per-error means ~50 new exported types in openapi3:
Want that, or cluster related cases into shared types with payload fields?
Clustering keeps the public API in the 10-15-type range while preserving type-based discrimination. 2. Common base How about embedding a small base type in all types so callers who don't care about specifics can errors.As the base? Happy to go either way. |
|
Yes for the common base, the idea is to be able to differentiate as needed using any of errors.Is .As .AsType. I think clustering where it makes sense is a great idea and is compatible with error wrapping: FieldVersionMismatchError would wrap the underlying LicenseIdentifierFieldFor31Plus error. I'm not sure I care about the names / lengths as long as the wrapping makes sense. |
Implements the design Pierre proposed in getkin#1166 (comment): per-call-site leaf types, clustered into family types via standard Go error wrapping, all reachable through a tiny ValidationError base via errors.As. Three layers of granularity, picked up at the caller's preferred level: - Base: *ValidationError. Catchall, just carries the human-readable Message. Reachable from any leaf via the As method each leaf implements. - Cluster: *RequiredFieldError, *FieldVersionMismatchError. Group families of related failures and expose family-level metadata (Field, MinVersion, ...). Wrap the leaf via Unwrap so errors.As walks past the cluster to the leaf. - Leaf: one type per call site (*InfoVersionRequired, *InfoTitleRequired, *LicenseIdentifierFieldFor31Plus, *InfoSummaryFieldFor31Plus). Lets callers match an exact failure point without string comparison. Demonstration set converts: - Info.Validate's two errors.New sites (info.title, info.version required) -> *RequiredFieldError wrapping their leaves. - errFieldFor31Plus dispatch -> *FieldVersionMismatchError wrapping a typed leaf when the field is in the switch (identifier, summary), or a bare *ValidationError otherwise. The fallback path means every existing errFieldFor31Plus call site immediately gains the cluster + base layers without touching its source — only the leaf type remains to be added. Backward compatibility: every converted site preserves its original Error() string byte-for-byte. Pinned by TestValidationError_BackwardCompatibleErrorString. Six new tests pin the design end-to-end: backward-compat strings, three-layer reachability for both clusters, leaf-vs-leaf discrimination, untyped-fallback behavior, MultiError flow-through. Bulk conversion of remaining ~50 call sites can land in follow-up PRs once this design is approved; each one is now a 1-2 line addition (a new leaf type + an entry in the relevant cluster's switch).
|
Pushed the design rework per your feedback (commit d1fb533). Three layers as discussed: Base — Clusters — Leaves — Demonstration set converts:
Six tests pin the design end-to-end: backward-compat strings (every converted site's Bulk conversion of the remaining ~50 sites can land in follow-up PRs once this shape is approved. |
Extends the per-type design demonstrated in d1fb533 to cover all errFieldFor31Plus dispatch (29 distinct fields: 4 direct + 25 via schema.go's reject() helper) and all "value of X must be a non-empty string" sites (5 distinct fields). Total: ~33 new exported leaf types, each ~5 lines (type + As method). Validator-side changes: - openapi3.go: errFieldFor31Plus dispatch table now covers webhooks, jsonschemadialect, summary, identifier, and all 25 schema.go reject() targets (const, examples, prefixItems, contains, minContains, maxContains, patternProperties, dependentSchemas, propertyNames, unevaluatedItems, unevaluatedProperties, if, then, else, dependentRequired, contentEncoding, contentMediaType, contentSchema, $defs, $schema, $comment, $id, $anchor, $dynamicAnchor, $dynamicRef). Fields not in the table fall back to a bare *ValidationError, so callers always get the cluster + base layers; only the leaf type is missing. - info.go: info.title, info.version → InfoTitleRequired, InfoVersionRequired (RequiredFieldError cluster). Already converted in d1fb533. - openapi3.go: doc.openapi → OpenAPIVersionRequired. - license.go: license.name → LicenseNameRequired. - server.go: server.url → ServerURLRequired. server_test.go: TestServerValidation switched from require.Equal on a plain errors.New(...) sentinel (which compared concrete types) to require.EqualError on the message string, which is the contract being preserved across the migration. Three new tests pin the bulk conversion: - TestValidationError_AllRequiredFieldLeaves: table-driven across openapi-version-required, license-name-required, server-url-required. Asserts cluster (RequiredFieldError + correct Field), specific leaf, and base ValidationError + correct Message at each. - TestValidationError_SchemaFieldFor31PlusLeaves: spot-checks ConstFieldFor31Plus and PatternPropertiesFieldFor31Plus, demonstrating that schema.go reject() targets gain the typed leaf via the dispatch table without any change to schema.go itself. - (Existing) TestValidationError_FieldVersionMismatch_UntypedFallback remains as the contract for the bare *ValidationError fallback path on fields not in the dispatch table. Backward compatibility unchanged: every converted site preserves its original Error() string byte-for-byte. Pinned by the existing TestValidationError_BackwardCompatibleErrorString plus the per-leaf tests' Message assertions.
|
Bulked the conversion in (commit 8cec548) — no follow-up PRs needed. Coverage:
That's ~33 new exported leaf types, each ~5 lines (type + As method). Test coverage:
One pre-existing test needed an update: Full |
|
Looks OK so far. Please have your tests use https://pkg.go.dev/errors#Unwrap and demonstrate the clustering. |
Per review feedback: use errors.Unwrap directly to show the wrap chain. New test exercises both RequiredFieldError -> leaf and FieldVersionMismatchError -> leaf, plus terminal-leaf check.
|
Done in 8fb8e07. New |
…dFor31Plus sites The 4 errFieldFor31Plus call sites with a literal field name (info.go, license.go, openapi3.go x 2) now name their leaf type at the call site instead of going through the string-keyed dispatch table. Same shape as the existing newInfoVersionRequired etc. for the RequiredFieldError cluster. The dispatch table now only contains schema-keyword fields, which are the only ones reached at runtime through schema.go's reject closure. errFieldFor31Plus stays as the helper that closure goes through.
|
Two more small commits since the test additions:
|
|
By the way to test usage of errors.AsType and since this repo uses go 1.25, you can write a test that mentions |
Build-tagged go1.26 since errors.AsType lands in 1.26. Demonstrates the generic complement to errors.As across cluster, leaf, and base on the validation-error hierarchy.
|
Done in b72a26f. New |
paths.Validate and PathItem.validateOperations both wrapped inner errors with %v, which builds the human-readable message but breaks errors.Is/As walking. Switch to %w so typed errors emitted by nested validators (schema-level *FieldVersionMismatchError leaves, etc.) remain reachable via errors.As from the top-level MultiError.
|
One more in 0879509: switched No behaviour change for anyone matching on |
fenollp
left a comment
There was a problem hiding this comment.
LGTM. Do you want me to merge this now or are you planning on adding commits?
RequiredFieldError and FieldVersionMismatchError gain an Origin *Origin field carrying the source location of the offending element (populated by the loader when IncludeOrigin is enabled). Constructors take the Origin from the relevant element: - info.Validate sites: info.Origin - license.Validate sites: license.Origin - server.Validate site: server.Origin - schema.go reject closure: schema.Origin Document-root fields (openapi version, webhooks, jsonSchemaDialect) have no Origin since the loader doesn't track *T; their constructors pass nil. Three new tests pin the contract: Origin populated when IncludeOrigin=true, nil when off, nil for doc-root fields regardless.
|
One more in 3b83447: Three tests pin the contract: populated with |
When a schema's example or default value doesn't satisfy the schema's
own constraints, validators previously wrapped the *SchemaError from
VisitJSON with fmt.Errorf("invalid example: %w", err) etc. Errors.As
could reach the inner *SchemaError but consumers couldn't tell which
sub-field's value failed without parsing the message prefix.
Add a *SchemaValueError cluster that carries:
- ValueKind: "example" or "default"
- Cause: the underlying *SchemaError or MultiError of them
- Origin: the offending element's source location (when tracking)
Convert four call sites: parameter.go, media_type.go, and two in
schema.go (default + example). Error() preserves the
"invalid example/default: <inner>" message format.
Test exercises the cluster on a parameter whose example exceeds the
schema's maxLength: cluster reachable via errors.As, *SchemaError
reachable via the Unwrap chain, Origin populated, message format
preserved.
|
One more in 3c0b649: added Converts four call sites: |
…ches paths.go:103 returned a plain fmt.Errorf when an operation declares fewer or more path-parameter declarations than the path template expects. Wrap in a typed cluster carrying Path, Method, Missing[], and Origin so consumers can dispatch by the path-parameter-mismatch shape without parsing the message.
|
Calling this PR feature-complete and ready for final review. What's in
Tests cover all three layers (cluster / leaf / base) via What's not in (the long tail)Roughly 80–100 Plan from hereMerge this PR; bulk-convert the long tail in follow-up PRs grouped by package (one for security_scheme, one for response/header, etc.). Two reasons:
Each follow-up will reuse the existing clusters where they fit (most "X is missing" cases land under Happy to address any final review nits here. Otherwise, ready when you are. |
Follow-up to getkin#1166 covering 7 long-tail untyped errors that fit the existing RequiredFieldError cluster shape: - externalDocs.url - operation.responses - requestBody.content - response.description - oAuthFlow.scopes - oAuthFlow.authorizationUrl - oAuthFlow.tokenUrl Each gets a leaf type, a constructor populating Origin from the relevant element, and a converted call site. operation_test.go switches from require.Equal-against-errors.New() to require.EqualError on the message string. Two representative tests pin cluster + leaf + base reachability.
* feat(openapi3): typed context errors for Validate() wrapper chain
Replaces the 14 fmt.Errorf wrap sites in Validate() with three typed
error types carrying their context as structured fields:
- SectionContextError{Section, Cause} — wraps an error inside one of
the top-level document sections (info, paths, components, security,
servers, tags, externalDocs, webhooks, jsonSchemaDialect)
- PathContextError{Path, Cause} — wraps an error inside a
specific path
- OperationContextError{Method, Cause} — wraps an error inside a
specific HTTP-method operation
Continues the typed-validation-error work from #1166 and #1180:
those PRs typed the leaf errors; this one types the wrapper layers
that carry doc-tree position around them.
Why: today, callers that want to render context separately (which
section a finding lives in, which path, which operation) have to
parse the rendered error string with regex. errors.As against typed
wrappers is the structured equivalent.
Backward compatibility: Error() strings are byte-identical to the
fmt.Errorf wrappers they replace. Existing consumers parsing the
rendered text continue to work unchanged. The typed extraction is
purely additive.
Wrap sites converted:
- openapi3.go: 9 sections (info, paths, components, security,
servers, tags, externalDocs, webhooks, jsonSchemaDialect)
- paths.go: per-path wrapper (PathContextError)
- path_item.go: per-operation wrapper (OperationContextError)
- operation.go, tag.go, schema.go: 3 additional externalDocs sites
(also use SectionContextError{Section: "external docs"})
Tests cover Error() format byte-stability, Unwrap chain walking,
errors.As extraction from a three-layer chain (section + path +
operation), and arbitrary non-typed inner causes. All existing
tests pass unchanged across openapi2, openapi3, openapi3conv,
openapi3filter, openapi3gen, and the routers — confirming the
rendered strings are stable.
.github/docs/openapi3.txt regenerated via docs.sh.
* refactor(openapi3): rename context errors to *ValidationError
Addresses review feedback on #1183:
- SectionContextError -> SectionValidationError
- PathContextError -> PathValidationError
- OperationContextError -> OperationValidationError
The new names tie these positional wrappers to the ValidationError
family they belong to (base ValidationError, cluster types, leaves).
Also renames the file to sit in that namespace:
section_context_error.go -> validation_error_context.go (+ _test.go).
No behavior change: Error() strings and Unwrap() chains are untouched.
.github/docs/openapi3.txt regenerated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Motivation
The document validation walker (
T.Validate,Info.Validate,Paths.Validate,Operation.Validate, etc.) returns plainerrors.New(...)values for ~50 distinct failure modes. Downstream code that wants to react to a specific failure has to compare on the human-readable message string:Brittle: any upstream rewording silently breaks the consumer. There's no current way to map from an error to a stable identifier.
This shows up concretely in tools that aggregate findings across many specs — diff/lint tools, governance dashboards, CI integrations — where users need a stable rule ID to suppress (
--ignore-rules) or filter on. Today they're forced into regex-bucket workarounds against the message text.What this PR adds
A typed
*ValidationErrorcarrying a stableCode(kebab-case subject-action, e.g.info-version-required) alongside the existingMessage:Then convert each
errors.New(msg)site in the validate paths to wrap the message:This commit converts only the two sites in
info.goto demonstrate the migration pattern. Bulk conversion of the remaining ~50 sites is deferred until the design settles.Backward compatibility
Fully additive — no existing behavior changes. Three pinned in tests:
err.Error() == \"value of version must be a non-empty string\"(*ValidationError).Error()returns the original byte-identical messageif err != nil { ... }errors.As(err, &target)*ValidationError); still works for everything that was working beforeMultiErrortraversalMultiError.As()already recurses into elements (openapi3/errors.go:41-49); typed errors flow through naturallyThree new tests pin all three:
TestValidationError_BackwardCompatibleErrorString—err.Error()returns the same string as before for both info-Validate failure modesTestValidationError_StructuredCodeViaErrorsAs—errors.As(err, &ve)succeeds and yields the expected CodeTestValidationError_FlowsThroughMultiError—errors.Asreaches into a MultiError to find a wrapped*ValidationErrorFull
go test ./...is green; no changes outside the new file plus the two-site demonstration ininfo.go.Precedent
openapi3already exposes typed errors (*SchemaError, used internally witherrors.Asat three sites inschema.go) and a public sentinel (ErrURINotSupportedinloader_uri_reader.go).*ValidationErroris in the same direction — additive, not a paradigm shift.Open questions
string. Alternative: a typedtype ValidationCode stringplus a const block (const InfoVersionRequired ValidationCode = \"info-version-required\"). Typed-constant gives compile-time autocomplete and grep-ability but adds maintenance surface. Bare string is more flexible. I defaulted to bare; happy to switch if you prefer.<subject>-<action>kebab-case (modeled after oasdiff's rule IDs and GitHub Actions::error::titles). Open to alternatives —OAS-XXXXnumeric, dotted (oas.info.version.required), etc.