kean / Get

Web API client built using async/await
MIT License
937 stars 74 forks source link

How to handle 422 json error response? #69

Open sjmueller opened 1 year ago

sjmueller commented 1 year ago

Our api server gives a 422 response when validating request params:

{
    "errors": [{ "detail": "Missing field: anon_id" }]
}

However, Get only supplies the error code: Response status code was unacceptable: 422 via APIError

.unacceptableStatusCode(let statusCode)

How can we hook into the actual error response for logging and (sometimes) showing to the user?

kean commented 1 year ago

You can throw a custom error by implementing client(_:validateResponse:data:task:).

It might be worth also expanding unacceptableStatusCode and adding response Data and URLResponse for convenience.

sjmueller commented 1 year ago

Thanks @kean, client(_:validateResponse:data:task:) is useful for the scenario of logging all error messages, bug reporting, etc.

I think you're on to something by expanding Data / URLResponse, because api.send catch block would be the ideal place to surface validation errors to the user.

kean commented 1 year ago

Thanks @kean, client(_:validateResponse:data:task:) is useful for the scenario of logging all error messages, bug reporting, etc.

It is also designed for providing a way to generate custom errors instead of Get.APIError. The APIs typically use the same error model across the endpoints, hence the centralized place for parsing them. But if the error models are different, it's not ideal, yes.

kean commented 1 year ago

I'm prototyping an improved version of async API for Get 3.0 to address Xcode 14.3 warnings.

I think it'll work nicely for decoding custom errors.

let dataTask = await client.dataTask(with: Request<User>(path: "/user"))

if #available(iOS 15, *) {
    for await progress in dataTask.progress.values {
        print(progress)
    }
}

do {
    let response = try await dataTask.response.value
} catch {
    let data = try await dataTask.data
    // Parse error
}

It could probably be improved further. Currently, if the server responds with 422, the entire Response is lost which is not ideal because you might need any of this information, even in case of a failure – as you pointed out. If I go in this direction, the response getter becomes non throwing, which I think will be ideal.

let response = await dataTask.response

do {
    let value = try await response.value // Decodes the value
} catch {
   // Catch all errors here
   if let data = response.data {
      // You change to parses data
   }
}