Expose response HTTP status code in errors #52
Conversation
| ) | ||
|
|
||
| func shouldNotBearAnHTTPStatusCode(actual interface{}, expected ...interface{}) string { | ||
| if code, present := fargo.HTTPResponseStatusCode(actual.(error)); present { | ||
| return fmt.Sprintf("Expected: no HTTP status code\nActual: %d", code) |
There was a problem hiding this comment.
I misinterpreted what I saw here in the assertion failure messages used by GoConvey, which I was trying to imitate. I'll fix it, aligning the fields with spaces instead.
| ) | ||
|
|
||
| type unsuccessfulHTTPResponse struct { | ||
| statusCode int |
There was a problem hiding this comment.
I would unify order with string output. Maybe message prefix first and rcode second?
There was a problem hiding this comment.
I wrote the struct in order from most important to least important, from required to optional, thinking that the response code is the type's _raison d'être _. I tried to keep the string format consistent with the rest of fargo's error messages. I don't think you're advocating changing the string format.
I could swap the field order. I just wanted you to know that its current arrangement was a deliberate choice.
There was a problem hiding this comment.
Please take a look at commit 4f48c58, in which I make sure to treat the message prefix as optional. Given that, let me know if you'd still prefer the field order to be swapped.
There was a problem hiding this comment.
I see. If you feel it's more appropriate leave it. I was just feeling that's it might look better when creating new error to have message first, so it would be similar to printf function. I'll leave decision to you. Thanks for clarifying.
| code, present := HTTPResponseStatusCode(test.input) | ||
| if present { | ||
| if !test.present { | ||
| t.Errorf("input %v: want absent, got code %d", test.input, code) |
There was a problem hiding this comment.
Can you please do those tests using Convey. (see other test files for guideline)
For various operations involving HTTP requests against the Eureka server, allow callers to interpret surfaced failures by way of inspecting the HTTP response's status code.
seh
force-pushed
the
expose-http-status-code-in-errors
branch
from
c913d16 to
648f74e
Compare
|
I'm happy enough with it. I'm going to implement some of #47 on the caller side to see if any additional helper functions emerge that are worth sharing, but I understand your goal of keeping the surface area of fargo small. |
For various operations involving HTTP requests against the Eureka server, allow callers to interpret surfaced failures by way of inspecting the HTTP response's status code.
This proposal is a subset of the preceding #47, also intended to address #45. Rather than fargo interpreting what the server's responses mean in the context of the attempted operation, we leave that interpretation open to callers.