[Feature request] Add `response.isOk` to check if the response status code is 2xx

[petrpavlik]

I believe that the community would benefit from adding a simple boolean check whether the status code of the response is acceptable.

I took inspiration from the javascript's fetch API where the response exposes response.ok to check whether the status code is >= 200 and < 300 (see the documentation), and created a simple extension to make my life easier.

extension HTTPClientResponse {
    var isOk: Bool {
        status.code >= 200 && status.code < 300
    }
}

Happy to contribute (and write tests) this if there is interest from the side of the maintainers.

[Lukasa]

I don't have any particular objection to this extension, though I'm not sure how useful it actually is: 3xx responses aren't errors, and many 2xx responses are conceptually not equivalent to ok. But the weight of fetch matters a fair bit here.

[petrpavlik]

I think that it's just a convenience abstraction over whether a successful request returns 200, 201, or what. That's how I've been using it.

My response handling usually looks something like this

if response.isOk {
  // try to parse the expected JSON
} else {
 // get the status code and try to parse the expected error JSON for 400 errors
}

This is for cases where I do not expect that a request may respond with a 3xx redirect.

[petrpavlik]

I'll close this for now since I haven't seen enough demand for this feature.

[MahdiBM]

I honestly do think this does have a lot of use cases and demand.
I use something like this basically anywhere I do HTTP requests.

Examples: in penny-bot, in DiscordBM.

3xx responses aren't errors

Right but I don't think there is enough demand for adding a 3xx method although I wouldn't mind it.
Usually HTTPClient handles the redirects itself as well, so 3xx rarely ever reaches the end user.

many 2xx responses are conceptually not equivalent to ok

I agree, maybe a var named is2xx would be more appropriate.

Considering this is trivial to do for end users, I'm not sure if in the end it'll be worth it to add.
One can just write 200..<300 ~= response.status.code or (200..<300).contains(response.status.code).

[petrpavlik]

@MahdiBM yeah, no problem at all, even though I feel like your code examples kind of do back me up that maybe this would make sense to wrap in a method and add :).

[0xifarouk]

HTTPTypes has Kind, can we have something similar? @Lukasa

https://github.com/apple/swift-http-types/blob/8e01d32d18cd2c7a188fc7613dae48426dbd7ddc/Sources/HTTPTypes/HTTPResponse.swift#L55

[petrpavlik]

That’s cool. There are some discussions about making this repo adapt HTTPTypes, but it’d be a lot of work.

[Lukasa]

Yes, we can definitely add kind as a compatibility bridge in swift-nio's NIOHTTP1 package.