Skip to content

PassthroughErrorHandler shouldn't pass an error if response is non-nil #156

Description

@ericleb010

The README for this package reads:

[...] if an error is returned by the client (connection errors, etc.), or if a 500-range response code is received (except 501), then a retry is invoked after a wait period. Otherwise, the response is returned and left to the caller to interpret.

However, as it stands, PassthroughErrorHandler doesn't seem to behave in a way that would accomplish this behaviour out of the box under normal circumstances. This is because it always passes to the caller of RoundTrip the final http.Response and the error from the call to CheckRetry. A default http.Client will actually throw away the response if an error is returned. So in the case where, for example, we exhaust all RetryMax attempts due to consecutive 5xx errors, the caller will never see the final response body.

PassthroughErrorHandler could be much more useful if it first checks for a non-nil response:

func PassthroughErrorHandler(resp *http.Response, err error, _ int) (*http.Response, error) {
	if resp != nil {
		return resp, nil
	}
	return resp, err
}

This throws away the helpful CheckRetry error on the last request, but gives the caller the expected ability to distinguish unexpected HTTP status codes from connection errors. Or maybe I'm misinterpreting what purpose PassthroughErrorHandler has?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions