Submit handles non 2xx responses#793
Conversation
cmd/submit.go
Outdated
| } | ||
|
|
||
| if resp.StatusCode < 200 || resp.StatusCode > 299 { | ||
| return fmt.Errorf("submission unsuccessful (%s)", resp.Status) |
There was a problem hiding this comment.
I'm guessing we want this to be more useful; what should this say?
There was a problem hiding this comment.
The API should respond with {"error": {"type": "some_value", "message": "A human-readable message"}} in the case of an error. Let's use the message if it has something available in it, otherwise let's use the default that you have here.
There was a problem hiding this comment.
Actually, you wrote code that fixed it. I've extracted that here:
#814
jdsutherland
force-pushed
the
submit-handles-non2xx
branch
2 times, most recently
from
8f6551f to
4b23582
Compare
|
@jdsutherland Now that we've gotten some of the helpers into master, would it make sense to rework this to use the helpers? |
jdsutherland
force-pushed
the
submit-handles-non2xx
branch
2 times, most recently
from
1c3d8a9 to
4b23582
Compare
Previously, 404s silently continue to misleading success output
jdsutherland
force-pushed
the
submit-handles-non2xx
branch
from
4b23582 to
10aea95
Compare
|
This is ready for another look. |
kytrinyx
left a comment
kytrinyx
left a comment
There was a problem hiding this comment.
This looks great! Thanks for your patience here 💙
| assert.Regexp(t, "test error", err.Error()) | ||
| } | ||
|
|
||
| func TestHandleErrorResponse(t *testing.T) { |
There was a problem hiding this comment.
Thank you for adding this test!
2 participants
Previously, 404s silently continue to misleading success output. This change throws an error for all non 2xx responses.
Closes #790.