fix: Preserve HTTP Response in URL Errors (#3369)

Fixes: #3362.

Author: silasalberti

github/github.go

@@ -841,28 +841,31 @@ 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:
 		}
 
 		// If the error type is *url.Error, sanitize its URL before returning.
 		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") == "" {

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) {