Propogate Do() HTTP error up when retries are exceeded #70
Conversation
|
|
theckman
force-pushed
the
return_err
branch
4 times, most recently
from
e5a3fae to
9d0ede6
Compare
|
@evanphx I think this is ready for review if you're interested / when you have a moment. |
|
Wouldn't it be better if we change the returned error to a typed model (instead of Also it would be nice that if there is a non-successful status code we add to the error the response body, considering the server may return a more detailed error description. |
theckman
force-pushed
the
return_err
branch
3 times, most recently
from
237475a to
4fbec8c
Compare
|
@tgropper would you be able to clarify what you mean by a "typed model"? I'm not familiar with that term. To your second point, could you clarify that a bit more? Are you suggesting we read the response body and append it to the error value we return? |
theckman
force-pushed
the
return_err
branch
2 times, most recently
from
613620c to
4819acc
Compare
theckman
force-pushed
the
return_err
branch
2 times, most recently
from
8326782 to
494eb9a
Compare
|
@evanphx this is ready for review, if you're still willing/able. |
Add propagation of the HTTP request/response error up the call stack when the number of retries are exceeded. Because the ErrorHandler does not receive a copy of the request, it's impossible to use the ErrorHandler to generate an enhanced version of the default Retries Exceeded error message. This change takes the non-API breaking approach to solving this, by including the error string in the returned error (if an error has occurred). Because the error string returned from `Do()` uses `%w`, this change requires Go 1.13+. Fixes hashicorp#69
|
LGTM too, merging! Thanks! |
* Realign behavior of ErrorPropagatedRetryPolicy with DefaultRetryPolicy ErrorPropagatedRetryPolicy should have been modified in #100, but failed to... probably because it has been merged approximately at the same time as #70 (the PR adding ErrorPropagatedRetryPolicy). * Remove code duplication between DefaultRetryPolicy and ErrorPropagatedRetryPolicy
Add propagation of the HTTP request/response error up the call stack when the
number of retries are exceeded. Because the ErrorHandler does not receive a copy
of the request, it's impossible to use the ErrorHandler to generate an enhanced
version of the default Retries Exceeded error message.
This change takes the non-API breaking approach to solving this, by including
the error string in the returned error (if an error has occured).
Fixes #69