-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Description
I are trying to make this library play nicely with a custom http.Client that converts certain error responses to errors at a lower level. This (rightly) breaks some things, because the error type this Client returns isn't an error type that go-github recognizes. One way to solve this would be to errors.Join the custom error type with an *ErrorResponse instance, so that it acts as both error types.
The problem I've faced is that go-github sometimes type-asserts errors down to *ErrorResponse, instead of using errors.As:
Line 1184 in f137c94
v, ok := target.(*ErrorResponse) Line 1489 in f137c94
if err, ok := err.(*ErrorResponse); ok && err.Response.StatusCode == http.StatusNotFound { Line 2370 in f137c94
errorResponse, ok := err.(*ErrorResponse)
This is not a bug in the library, because our http.Client implementation technically breaks the API contract of http.Client; the doc comment above RoundTripper.RoundTrip says:
// RoundTrip should not attempt to interpret the response. In
// particular, RoundTrip must return err == nil if it obtained
// a response, regardless of the response's HTTP status code.This pattern of customizing errors at the http.Client level is nonetheless useful to us, and it would be helpful if go-github played nicely with it. I believe the only changes required would be to use errors.As instead of a direct type assertion at the three locations I mentioned above. Would you be accepting of such a change?
Thanks for your time.