diff --git a/README.md b/README.md index 40a9b08..40504c7 100644 --- a/README.md +++ b/README.md @@ -215,8 +215,10 @@ if you want each entry to carry a `value` field. The OpenAPI spec exposes `ErrorEnvelope` / `ValidationErrorEntry` / `ResponseContext` under `components.schemas` and adds a 422 response with a realistic example to every operation, plus a 400 example for body-carrying methods. Routes that -declare a non-empty `TError` keep their domain shape under the catch-all `4XX` -response. +declare a non-empty `TError` and no per-status `OpenAPIOptions.Errors` keep their +domain shape under the catch-all `4XX` response — as soon as the route declares +explicit `Errors` entries the catch-all is suppressed and only the enumerated +status codes appear in the spec. If you need a different shape, set `Config.ValidationErrorHandler` / `Config.AuthErrorHandler` — they receive the raw error (JSON type mismatches are wrapped so `err.Error()` diff --git a/custom_errors.go b/custom_errors.go index d743d04..2c6184b 100644 --- a/custom_errors.go +++ b/custom_errors.go @@ -139,6 +139,19 @@ func statusCodeKey(code int) string { return strconv.Itoa(code) } +// hasNonNilErrorEntry reports whether the slice contains at least one non-nil +// entry. A slice of only nils is treated as "no errors declared" since the +// spec-generation loop skips nil entries — counting them as a declaration +// would suppress the 4XX catch-all without emitting any replacement. +func hasNonNilErrorEntry(errors []any) bool { + for _, e := range errors { + if e != nil { + return true + } + } + return false +} + // errorCategory groups the inputs needed to materialise a user-defined error // shape from a library-internal error. The category is what we know about the // error before we know what shape the user wants. diff --git a/custom_errors_test.go b/custom_errors_test.go index 012e7fa..bf05c93 100644 --- a/custom_errors_test.go +++ b/custom_errors_test.go @@ -213,6 +213,79 @@ func TestCustomErrors_NilErrorReturnsSuccess(t *testing.T) { assert.Equal(t, "ok", out.Message) } +// legacyTError is a non-empty struct used to exercise the TError catch-all +// behaviour in the next two tests. +type legacyTError struct { + Code int `json:"code"` + Message string `json:"message"` +} + +func TestCustomErrors_Suppresses4XXWhenErrorsDeclared(t *testing.T) { + // When OpenAPIOptions.Errors is populated, the legacy 4XX catch-all is + // redundant — the user has explicitly enumerated which status codes their + // handler can emit. The spec should list ONLY those concrete codes. + app := fiber.New() + oapi := New(app) + + Post(oapi, "/items/:name", func(c fiber.Ctx, input customErrInput) (customErrOutput, *legacyTError) { + return customErrOutput{Message: "ok"}, nil + }, OpenAPIOptions{ + OperationID: "createItem", + Errors: []any{appConflict("a"), appNotFound("b")}, + }) + + spec := oapi.GenerateOpenAPISpec() + responses := spec["paths"].(map[string]any)["/items/{name}"].(map[string]any)["post"].(map[string]any)["responses"].(map[string]any) + + _, has4xx := responses["4XX"] + assert.False(t, has4xx, "4XX must be suppressed when Errors[] is non-empty") + + // Sanity: the explicit codes are still there. + _, has409 := responses["409"] + _, has404 := responses["404"] + assert.True(t, has409 && has404, "the explicit Errors entries must still surface") +} + +func TestCustomErrors_4XXStillEmittedWhenErrorsSliceOnlyContainsNils(t *testing.T) { + // Edge case: Errors: []any{nil} should be treated as "nothing declared", + // not as "errors declared". The downstream emission loop skips nil entries, + // so if we suppressed the 4XX based on slice length the route would end up + // with zero documented error responses at all. + app := fiber.New() + oapi := New(app) + + Post(oapi, "/items/:name", func(c fiber.Ctx, input customErrInput) (customErrOutput, *legacyTError) { + return customErrOutput{Message: "ok"}, nil + }, OpenAPIOptions{ + OperationID: "createItem", + Errors: []any{nil, nil}, + }) + + spec := oapi.GenerateOpenAPISpec() + responses := spec["paths"].(map[string]any)["/items/{name}"].(map[string]any)["post"].(map[string]any)["responses"].(map[string]any) + + _, has4xx := responses["4XX"] + assert.True(t, has4xx, "4XX must still be emitted when the Errors slice contains only nil entries") +} + +func TestCustomErrors_4XXStillEmittedWhenNoErrorsDeclared(t *testing.T) { + // Backwards compatibility: routes whose handler declares a non-empty TError + // but provides no Errors[] entries still get the legacy 4XX catch-all so + // existing integrations are not silently broken. + app := fiber.New() + oapi := New(app) + + Post(oapi, "/items/:name", func(c fiber.Ctx, input customErrInput) (customErrOutput, *legacyTError) { + return customErrOutput{Message: "ok"}, nil + }, OpenAPIOptions{OperationID: "createItem"}) + + spec := oapi.GenerateOpenAPISpec() + responses := spec["paths"].(map[string]any)["/items/{name}"].(map[string]any)["post"].(map[string]any)["responses"].(map[string]any) + + _, has4xx := responses["4XX"] + assert.True(t, has4xx, "4XX must still be emitted when no Errors[] is declared (legacy behaviour)") +} + func TestCustomErrors_PrecedenceOverDefault404Envelope(t *testing.T) { // When the user declares a 404 in Errors AND has called UseNotFoundHandler(), // the declared shape (their AppError) wins for the per-route spec entry — diff --git a/fiberoapi.go b/fiberoapi.go index 97d7b04..6553003 100644 --- a/fiberoapi.go +++ b/fiberoapi.go @@ -400,9 +400,16 @@ func (o *OApiApp) GenerateOpenAPISpec() map[string]interface{} { } // Custom TError response — only when the handler returns a non-empty TError. - // Emitted as a 4xx response separate from the default validation envelope so - // callers see both shapes in the spec. - if op.ErrorType != nil && !isEmptyStruct(op.ErrorType) { + // Emitted as a 4XX catch-all so legacy users who do not declare per-status + // entries via OpenAPIOptions.Errors still get their domain error documented. + // + // When the route DOES declare at least one non-nil Errors entry, the 4XX + // is redundant (and worse, misleading): the user has explicitly enumerated + // the status codes their handler can emit, so the catch-all just pollutes + // the spec. Count non-nil entries — a slice that only contains nil is + // equivalent to no declaration since the emission loop below would skip + // every entry, leaving the route with zero documented error responses. + if op.ErrorType != nil && !isEmptyStruct(op.ErrorType) && !hasNonNilErrorEntry(op.Options.Errors) { errorType := dereferenceType(op.ErrorType) var schemaRef map[string]interface{}