diff --git a/github/github.go b/github/github.go index 6f476353473..4f94f2481a4 100644 --- a/github/github.go +++ b/github/github.go @@ -841,12 +841,17 @@ func (c *Client) BareDo(ctx context.Context, req *http.Request) (*Response, erro } resp, err := c.client.Do(req) + var response *Response + if resp != nil { + response = newResponse(resp) + } + if err != nil { // If we got an error, and the context has been canceled, // the context's error is probably more useful. select { case <-ctx.Done(): - return nil, ctx.Err() + return response, ctx.Err() default: } @@ -854,15 +859,13 @@ func (c *Client) BareDo(ctx context.Context, req *http.Request) (*Response, erro if e, ok := err.(*url.Error); ok { if url, err := url.Parse(e.URL); err == nil { e.URL = sanitizeURL(url).String() - return nil, e + return response, e } } - return nil, err + return response, err } - response := newResponse(resp) - // Don't update the rate limits if this was a cached response. // X-From-Cache is set by https://github.com/gregjones/httpcache if response.Header.Get("X-From-Cache") == "" { diff --git a/github/github_test.go b/github/github_test.go index 73afa2d1968..48ac2ab26e5 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -1106,6 +1106,51 @@ func TestDo_redirectLoop(t *testing.T) { } } +func TestDo_preservesResponseInHTTPError(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + fmt.Fprintf(w, `{ + "message": "Resource not found", + "documentation_url": "https://docs.github.com/rest/reference/repos#get-a-repository" + }`) + }) + + req, _ := client.NewRequest("GET", ".", nil) + var resp *Response + var data interface{} + resp, err := client.Do(context.Background(), req, &data) + + if err == nil { + t.Fatal("Expected error response") + } + + // Verify error type and access to status code + errResp, ok := err.(*ErrorResponse) + if !ok { + t.Fatalf("Expected *ErrorResponse error, got %T", err) + } + + // Verify status code is accessible from both Response and ErrorResponse + if resp == nil { + t.Fatal("Expected response to be returned even with error") + } + if got, want := resp.StatusCode, http.StatusNotFound; got != want { + t.Errorf("Response status = %d, want %d", got, want) + } + if got, want := errResp.Response.StatusCode, http.StatusNotFound; got != want { + t.Errorf("Error response status = %d, want %d", got, want) + } + + // Verify error contains proper message + if !strings.Contains(errResp.Message, "Resource not found") { + t.Errorf("Error message = %q, want to contain 'Resource not found'", errResp.Message) + } +} + // Test that an error caused by the internal http client's Do() function // does not leak the client secret. func TestDo_sanitizeURL(t *testing.T) {