Skip to content

Architecture: OpenAPI validator panics on invalid spec #12

@kusold

Description

@kusold

Description

The OpenAPI validator panics if the spec is invalid, crashing the application on startup instead of returning a clear error.

Location

// openapi/openapi.go
func Validator(spec []byte, cfg Config) func(next http.Handler) http.Handler {
    conf := cfg.withDefaults()

    document, err := libopenapi.NewDocument(spec)
    if err != nil {
        panic(err)  // <-- Panics instead of returning error
    }

    v, errs := validator.NewValidator(document)
    if errs != nil {
        panic(errs)  // <-- Panics instead of returning error
    }
    // ...
}

Problem

Panics are difficult to test and don't provide clear error messages for debugging. A malformed spec will crash the application with a stack trace.

Recommended Fix

Return an error instead of panicking:

func NewValidator(spec []byte, cfg Config) (func(http.Handler) http.Handler, error) {
    conf := cfg.withDefaults()

    document, err := libopenapi.NewDocument(spec)
    if err != nil {
        return nil, fmt.Errorf("invalid OpenAPI document: %w", err)
    }

    v, errs := validator.NewValidator(document)
    if errs != nil {
        return nil, fmt.Errorf("invalid OpenAPI spec: %v", errs)
    }

    // ... validation logic

    return func(next http.Handler) http.Handler {
        // middleware implementation
    }, nil
}

Update callers:

// In app initialization
validator, err := openapi.NewValidator(spec, cfg.OpenAPI)
if err != nil {
    return nil, fmt.Errorf("failed to create OpenAPI validator: %w", err)
}
router.Use(validator)

Alternative

If panicking is intentional (fail fast), document why and add a recovery mechanism:

// Validate panics if the spec is invalid. This is intentional to catch
// configuration errors at startup rather than runtime.
// Use openapi.ValidateSpec() to check validity without panicking.
func Validator(spec []byte, cfg Config) func(http.Handler) http.Handler {
    // ...
}

func ValidateSpec(spec []byte) error {
    // Non-panicking version for testing
}

Impact

  • Effort: Low
  • Benefit: Better error messages, testability
  • Breaking: Potentially (API change)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions