diff --git a/github/repos_contents.go b/github/repos_contents.go index 02d45a12ce0..782daf74226 100644 --- a/github/repos_contents.go +++ b/github/repos_contents.go @@ -117,26 +117,30 @@ func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string, // specified file. This function will work with files of any size, as opposed // to GetContents which is limited to 1 Mb files. It is the caller's // responsibility to close the ReadCloser. -func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, error) { +// +// It is possible for the download to result in a failed response when the +// returned error is nil. Callers should check the returned Response status +// code to verify the content is from a successful response. +func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, *Response, error) { dir := path.Dir(filepath) filename := path.Base(filepath) - _, dirContents, _, err := s.GetContents(ctx, owner, repo, dir, opts) + _, dirContents, resp, err := s.GetContents(ctx, owner, repo, dir, opts) if err != nil { - return nil, err + return nil, resp, err } for _, contents := range dirContents { if *contents.Name == filename { if contents.DownloadURL == nil || *contents.DownloadURL == "" { - return nil, fmt.Errorf("No download link found for %s", filepath) + return nil, resp, fmt.Errorf("No download link found for %s", filepath) } - resp, err := s.client.client.Get(*contents.DownloadURL) + dlResp, err := s.client.client.Get(*contents.DownloadURL) if err != nil { - return nil, err + return nil, &Response{Response: dlResp}, err } - return resp.Body, nil + return dlResp.Body, &Response{Response: dlResp}, nil } } - return nil, fmt.Errorf("No file named %s found in %s", filename, dir) + return nil, resp, fmt.Errorf("No file named %s found in %s", filename, dir) } // GetContents can return either the metadata and content of a single file diff --git a/github/repos_contents_test.go b/github/repos_contents_test.go index f122525800a..a0c4695c202 100644 --- a/github/repos_contents_test.go +++ b/github/repos_contents_test.go @@ -118,11 +118,15 @@ func TestRepositoriesService_DownloadContents_Success(t *testing.T) { fmt.Fprint(w, "foo") }) - r, err := client.Repositories.DownloadContents(context.Background(), "o", "r", "d/f", nil) + r, resp, err := client.Repositories.DownloadContents(context.Background(), "o", "r", "d/f", nil) if err != nil { t.Errorf("Repositories.DownloadContents returned error: %v", err) } + if got, want := resp.Response.StatusCode, http.StatusOK; got != want { + t.Errorf("Repositories.DownloadContents returned status code %v, want %v", got, want) + } + bytes, err := ioutil.ReadAll(r) if err != nil { t.Errorf("Error reading response body: %v", err) @@ -134,6 +138,43 @@ func TestRepositoriesService_DownloadContents_Success(t *testing.T) { } } +func TestRepositoriesService_DownloadContents_FailedResponse(t *testing.T) { + client, mux, serverURL, teardown := setup() + defer teardown() + mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `[{ + "type": "file", + "name": "f", + "download_url": "`+serverURL+baseURLPath+`/download/f" + }]`) + }) + mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, "foo error") + }) + + r, resp, err := client.Repositories.DownloadContents(context.Background(), "o", "r", "d/f", nil) + if err != nil { + t.Errorf("Repositories.DownloadContents returned error: %v", err) + } + + if got, want := resp.Response.StatusCode, http.StatusInternalServerError; got != want { + t.Errorf("Repositories.DownloadContents returned status code %v, want %v", got, want) + } + + bytes, err := ioutil.ReadAll(r) + if err != nil { + t.Errorf("Error reading response body: %v", err) + } + r.Close() + + if got, want := string(bytes), "foo error"; got != want { + t.Errorf("Repositories.DownloadContents returned %v, want %v", got, want) + } +} + func TestRepositoriesService_DownloadContents_NoDownloadURL(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -145,10 +186,14 @@ func TestRepositoriesService_DownloadContents_NoDownloadURL(t *testing.T) { }]`) }) - _, err := client.Repositories.DownloadContents(context.Background(), "o", "r", "d/f", nil) + _, resp, err := client.Repositories.DownloadContents(context.Background(), "o", "r", "d/f", nil) if err == nil { t.Errorf("Repositories.DownloadContents did not return expected error") } + + if resp == nil { + t.Errorf("Repositories.DownloadContents did not return expected response") + } } func TestRepositoriesService_DownloadContents_NoFile(t *testing.T) { @@ -159,10 +204,14 @@ func TestRepositoriesService_DownloadContents_NoFile(t *testing.T) { fmt.Fprint(w, `[]`) }) - _, err := client.Repositories.DownloadContents(context.Background(), "o", "r", "d/f", nil) + _, resp, err := client.Repositories.DownloadContents(context.Background(), "o", "r", "d/f", nil) if err == nil { t.Errorf("Repositories.DownloadContents did not return expected error") } + + if resp == nil { + t.Errorf("Repositories.DownloadContents did not return expected response") + } } func TestRepositoriesService_GetContents_File(t *testing.T) {