HTTPClient: log actual response status code
#2487
Conversation
We were currently logging the status code retrieved from `ETagManager`, which made it seem like etags weren't working. Example: > API request completed: GET /v1/subscribers/identify (200) With this change, we now log the actual response status code so we can verify the server is returning the correct status code: > API request completed: GET /v1/subscribers/identify (304)
Codecov Report
@@ Coverage Diff @@
## main #2487 +/- ##
==========================================
+ Coverage 86.64% 87.67% +1.02%
==========================================
Files 195 197 +2
Lines 13056 13303 +247
==========================================
+ Hits 11313 11664 +351
+ Misses 1743 1639 -104
|
| Logger.debug(Strings.network.api_request_completed(request.httpRequest, httpCode: response.statusCode)) | ||
| Logger.debug(Strings.network.api_request_completed( | ||
| request.httpRequest, | ||
| httpCode: urlResponse?.httpStatusCode ?? response.statusCode |
There was a problem hiding this comment.
let's add a comment, it's not obvious at a glance why we're checking them in this order
There was a problem hiding this comment.
or make it a method with a clear name or something
There was a problem hiding this comment.
Yup good point 👍🏻
| var httpStatusCode: HTTPStatusCode? { | ||
| guard let response = self as? HTTPURLResponse else { return nil } | ||
|
|
||
| return .init(rawValue: response.statusCode) |
There was a problem hiding this comment.
[nitpick, FFTI] it might just be me, but I find this pattern in general to be less readable than
return HTTPStatusCode(rawValue: response.statusCode)since I have to do a double-take and check what the return type of the method is to figure out what we're creating
There was a problem hiding this comment.
Yeah it's definitely a tradeoff, especially for more complex functions where the context of the type might be lost to the reader. In this particular case my thinking is just "let the compiler figure it out".
We were currently logging the status code retrieved from `ETagManager`, which made it seem like etags weren't working. Example: > API request completed: GET /v1/subscribers/identify (200) With this change, we now log the actual response status code so we can verify the server is returning the correct status code: > API request completed: GET /v1/subscribers/identify (304)
We were currently logging the status code retrieved from
ETagManager, which made it seem like etags weren't working.Example:
With this change, we now log the actual response status code so we can verify the server is returning the correct status code: