OpenAPI, trying to use SOAR-0011 ErrorHandlingMiddleware

Hi! I am trying to use the implementation of this proposal Middleware in conjunction with Hummingbird.

Have it working from my call site

    let clown_api = ClownAPI(repository: clownStore.repository)
    do {
        try clown_api.registerHandlers(on: router, serverURL: URL(string: clown_api.basePath)!, middlewares: [ErrorHandlingMiddleware()])
    } catch {
        //TODO: LOG THIS
        print("hello api failed to register. Those endpoints will not be available.")
    }

My API has the following put available:

/:
    get:
     #  ...
    post:
      operationId: create
      requestBody:
        description: a clown to create
        required: true
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/ClownCreateRequest"
      responses:
        "200":
          description: Successful creation with the clown returned.
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Clown"
        "201":
          description: Successful creation with empty body
        "400":
          description: Some bad request.
...
components:
  schemas:
    # ...
    ClownCreateRequest:
      type: object
      description: Fields to try to create a clown from.
      properties:
        name:
          type: string
        spareNoses:
          type: integer
      required:
        - name

And the create function:

extension Components.Schemas.Clown {
    /// Maps a `Clown` to a `Components.Schemas.Clown`
    /// This makes it easier to send models to the API
    init(clown: Clown) {
        self.init(id: clown.id, name: clown.name, spareNoses: clown.spareNoses)
    }
}
struct ClownAPI: APIProtocol {
    let basePath = "/api/v0/clowns"
    let repository: ClownCar
//...
    func create(_ input:Operations.create.Input) async throws -> Operations.create.Output {
        print("CREATING!!!")
        //ClownCreateRequest

        //This never got hit. 
        // guard case .json(let body) = input.body else {
        //     return .undocumented(statusCode: 422, .init())
        // }
        
        let (name, suggestedNoses) = switch input.body {
            case .json(let clownInfo):
                (clownInfo.name, clownInfo.spareNoses)
        }
        print(name, suggestedNoses ?? 0)
        let clown = try await repository.create(name: name, spareNoses: suggestedNoses)
        return .ok(.init(body: .json(.init(clown: clown))))
    }
}

I've run the following tests:

Successes:

  • 200 (success): curl -i -X POST localhost:8080/api/v0/clowns/ -d'{"name":"Daisy A. Eausauter","spareNoses":55}' -H 'Content-Type: application/json'
  • 200 (success, left out optional parameter): curl -i -X POST localhost:8080/api/v0/clowns/ -d'{"name":"Daisy B. Eausauter"}' -H 'Content-Type: application/json'
  • 200 (success, JSON had extra parameters, though.) curl -i -X POST localhost:8080/api/v0/clowns/ -d'{"name":"Daisy C. Eausauter","spareNoses":55, "aLittleSomething":"extra"}' -H 'Content-Type: application/json'

These newly do something other than 500:

  • 404 (not a defined route): curl -i -X POST localhost:8080/api/v0/clowns/undefinedRoute
  • 415 (JSON is right, but header is missing): curl -i -X POST localhost:8080/api/v0/clowns -d'{"name":"Daisy D. Eausauter","spareNoses":55}'

But these still 500's

  • 500 (has header, but JSON omits required): curl -i -X POST localhost:8080/api/v0/clowns/ -d'{"spareNoses":55}' -H 'Content-Type: application/json'
  • 500 (", value types are correct but keys are wrong) curl -i -X POST localhost:8080/api/v0/clowns/ -d'{"string":"Daisy E. Eausauter","number":55}' -H 'Content-Type: application/json'
  • 500 (", unrelated json entirely) curl -i -X POST localhost:8080/api/v0/clowns/ -d'{"whoKnows":"whatThisEvenIs","LongNumber":"fifty-five"}' -H 'Content-Type: application/json'
  • 500 (", keys correct values wrongly typed) curl -i -X POST localhost:8080/api/v0/clowns/ -d'{"name":12,"spareNoses":"none"}' -H 'Content-Type: application/json'

The inline documentation says to extend your App's error type to provide things to throw:

/// extension MyAppError: HTTPResponseConvertible {
///    var httpStatus: HTTPResponse.Status {
///        switch self {
///        case .invalidInputFormat:
///            .badRequest
///        case .authorizationError:
///            .forbidden
///        }
///    }
/// }

But I'm not even getting to my function in order to be able to throw something by hand.

How do I take advantage of this new feature and intercede on the errors?

Answered my own question.

Went back and re-read Issue #609 on GitHub

And realized that it wasn't MY error type that needed to be extended for those early errors but Hummingbirds. ETA: Nope correction again.. thats an extension on the decodable conformance??

extension DecodingError: @retroactive HTTPResponseConvertible {
    public var httpStatus: HTTPResponse.Status {
        switch self {
        
        case .typeMismatch(_, _):
                .unprocessableContent
        case .valueNotFound(_, _):
                .notFound
        case .keyNotFound(_, _):
                .badRequest
        case .dataCorrupted(_):
                .badRequest
        @unknown default:
                .badRequest
        }
    }
}

I'm still a little lost because I want to beable to pass a json/problem body too, but one step at a time!

ETA: Okay I have the start of something that can work

import OpenAPIRuntime
import Hummingbird

//This is conformance in order to work with OpenAPI's ErrorHandlingMiddleware()

extension DecodingError: @retroactive HTTPResponseConvertible {
    public var httpStatus: HTTPResponse.Status {
        switch self {
        
        case .typeMismatch(_, _):
                .unprocessableContent
        case .valueNotFound(_, _):
                .notFound
        case .keyNotFound(_, _):
                .badRequest
        case .dataCorrupted(_):
                .badRequest
        @unknown default:
                .badRequest
        }
    }

    public var httpBody: OpenAPIRuntime.HTTPBody? {
        switch self { 
        case .typeMismatch(let type, let deContext):
                //`[CodingKeys(stringValue: "name", intValue: nil)]`
                let error = "\(deContext.underlyingError?.localizedDescription ?? "")"
                let path = deContext.codingPath.description
                return .init("Type mismatch for `\(path)` key, expected `\(type)` type.\(error)")
        case .valueNotFound(let value, let deContext):
                let typeDescription = "\(value.self)"
                return .init("notFound: \(typeDescription) for \(deContext.codingPath)")
        case .keyNotFound(let key, _):
                let key = key.stringValue
                return .init( "keyNotFound: \(key)")
        case .dataCorrupted(let deContext):
                if let message = deContext.underlyingError?.localizedDescription {
                    return .init( "dataCorrupted due to error: \(message)")
                } else {
                    return .init( "dataCorrupted due to error in: \(deContext.codingPath)")
                }
        @unknown default:
                return .init( "unknown error: \(self.localizedDescription)")

        }

    }

    public  var httpHeaderFields: HTTPTypes.HTTPFields { 
        [.contentType: "text/plain"]
    }

}

Credit to : hummingbird/Sources/Hummingbird/Server/URI+decodeQuery.swift at 3ae359b1bb1e72378ed43b59fdcd4d44cac5d7a4 · hummingbird-project/hummingbird · GitHub

And I'm not sure if this will work / would work but tried out doing a Hummingbird specific conformance:

import Hummingbird 
import OpenAPIRuntime

extension HTTPError: @retroactive HTTPResponseConvertible {
    public var httpStatus: HTTPTypes.HTTPResponse.Status {
        self.status
    }

    public var httpHeaderFields: HTTPFields {
        self.headers
    }

    public var httpBody: HTTPBody? {
        if let data = self.body?.data(using: .utf8) {
            return HTTPBody(data)
        } 
        return nil
    }
    
}

If the request is failing to parse, your handler will never get called. The HTTPResponseConvertible protocol is supposed to be used when you want to throw custom errors from your handler or middlewares, and convert them to specific responses.

It does not allow you to change how the built-in decoding of requests reports its errors.

We're also tracking DecodingError should return 4xx instead of 500 · Issue #717 · apple/swift-openapi-generator · GitHub

1 Like

Are you saying I shouldn't have to change my decoding errors or the code I posted shouldn't work at all... because it does and the Decoding Errors work just the way I want them to now (although in need of refinement). Honestly, those were the errors I went looking to change.

I think if Hummingbird conforms HTTPError to HTTPResponseConvertible, its default decoding errors might just start working but I haven't actually tested that.

But I'm hearing the fact that I could override those was an unexpected result? Tomorrow I'll try pulling out the middleware and see if it's just the conformance that's doing it.

Happy to see those 500's under discussion. (a 5XX in response to a client submitted data problem is not what 5xx errors mean?). I would love for the OpenAPI Generator to provide better default codes for those early errors because get 3 web-devs in the room and there's 8 opinions about what to use.

That said, shouldn't the OpenAPI yaml file get the final say? If someone wants to throw a different error with a different response body because that's what their clients currently expect or simply what they prefer (422 for type mismatch, 200 for everything with a response body, these are all out there) are you saying that the generator shouldn't allow for overrides?

That's probably fine for new API's but someone trying to port an existing server to Swift without nuking all their existing clients would find themselves in a pickle trying to use it.

Are you saying I shouldn't have to change my decoding errors or the code I posted shouldn't work at all... because it does and the Decoding Errors work just the way I want them to now (although in need of refinement). Honestly, those were the errors I went looking to change.

Not exactly, I’m just saying that any errors that already conform to HTTPResponseConvertible, you can’t change how they convert. But any errors that don’t conform yet, and you own them, you should totally feel free to conform them. That includes opening a PR to the Hummingbird OpenAPI transport repo to conform HTTPError - that project is owned by the Hummingbird team, not by the Swift OpenAPI Generator team, so you’ll want to discuss the details with them.

I would love for the OpenAPI Generator to provide better default codes for those early errors

Agreed, that’s the goal behind that issue. If you’re open to contributing a fix here, we’d welcome a PR.

That said, shouldn't the OpenAPI yaml file get the final say?

The OpenAPI doc gets the final say for errors thrown out of your handlers, since you return the exact response documented in the OpenAPI document. The HTTPResponseConvertible feature is just a convenience to allow for errors thrown out of utility functions to still provide reasonable response codes, rather than just the undifferentiated 500.

But keep in mind that errors thrown before your user handler is invoked, and errors thrown by your user handler, are quite different. The former is thrown by the generated code, when the request does not comply with the OpenAPI document, and those errors cannot be overridden by individual users (we just want to improve the codes they produce, tracked by the linked issue). The latter are completely in the user’s control, you can return any HTTP code from your user handler, while of course we recommend you to return a documented HTTP response.

What a great detailed reply!

Oh, I've been knocking on the Hummingbird folks door quite a bit the past couple of weeks and they've been absolutely lovely about it. That inline link was to an issue already filed.

Thank you for the invitation to try a PR! Maybe if the issue is still open in November? I am under a fairly tight deadline at the moment and the server is just one piece of a bigger IoT project and I have to switch to hardware by the first. TBH, though, my help may be more trouble than its worth as I honestly think each handler should have the opportunity to handle its own decoding problems...

I'll admit I am still confused/intrigued when you say:

and those errors cannot be overridden by individual users

But... I did. I did override them. Simply by providing a extension DecodingError: @retroactive HTTPResponseConvertible in my code... It took over. And I wasn't the first. I stole the start of that code from issue #609 linked it the OP.

These are errors from before the handler...

And I'm happy about that.

Is this a bug? That will be taken away? That would be disappointing. How am I supposed to write appropriate server specific Problem Details if I can't change the http errors?

I’d be very careful about doing that, my read of swift-evolution/proposals/0364-retroactive-conformance-warning.md at main · swiftlang/swift-evolution · GitHub sentence “Before the client removes their conformance and rebuilds, however, their application will exhibit undefined behavior, as it is indeterminate which definition of this conformance will "win".” is that you’ve landed in undefined behavior land, and you might get different behavior each time. Maybe @harlanhaskins can advise more here.

2 Likes