diff --git a/github/github.go b/github/github.go index b9a2488aaf5..6de7174b925 100644 --- a/github/github.go +++ b/github/github.go @@ -723,9 +723,8 @@ func (e *Error) Error() string { // CheckResponse checks the API response for errors, and returns them if // present. A response is considered an error if it has a status code outside // the 200 range or equal to 202 Accepted. -// API error responses are expected to have either no response -// body, or a JSON response body that maps to ErrorResponse. Any other -// response body will be silently ignored. +// API error responses are expected to have response +// body, and a JSON response body that maps to ErrorResponse. // // The error type will be *RateLimitError for rate limit exceeded errors, // *AcceptedError for 202 Accepted status codes, @@ -742,6 +741,10 @@ func CheckResponse(r *http.Response) error { if err == nil && data != nil { json.Unmarshal(data, errorResponse) } + // Re-populate error response body because GitHub error responses are often + // undocumented and inconsistent. + // Issue #1136, #540. + r.Body = ioutil.NopCloser(bytes.NewBuffer(data)) switch { case r.StatusCode == http.StatusUnauthorized && strings.HasPrefix(r.Header.Get(headerOTP), "required"): return (*TwoFactorAuthError)(errorResponse) diff --git a/github/github_test.go b/github/github_test.go index 1aa3ef9b1bb..2bde0ac3b5d 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -942,6 +942,36 @@ func TestCheckResponse_noBody(t *testing.T) { } } +func TestCheckResponse_unexpectedErrorStructure(t *testing.T) { + httpBody := `{"message":"m", "errors": ["error 1"]}` + res := &http.Response{ + Request: &http.Request{}, + StatusCode: http.StatusBadRequest, + Body: ioutil.NopCloser(strings.NewReader(httpBody)), + } + err := CheckResponse(res).(*ErrorResponse) + + if err == nil { + t.Errorf("Expected error response.") + } + + want := &ErrorResponse{ + Response: res, + Message: "m", + Errors: []Error{{}}, + } + if !reflect.DeepEqual(err, want) { + t.Errorf("Error = %#v, want %#v", err, want) + } + data, err2 := ioutil.ReadAll(err.Response.Body) + if err2 != nil { + t.Fatalf("failed to read response body: %v", err) + } + if got := string(data); got != httpBody { + t.Errorf("ErrorResponse.Response.Body = %q, want %q", got, httpBody) + } +} + func TestParseBooleanResponse_true(t *testing.T) { result, err := parseBoolResponse(nil) if err != nil {