From c52429d8984818e4305fb89f7dda7a04b149cee1 Mon Sep 17 00:00:00 2001 From: Tamas Papik Date: Tue, 13 Feb 2024 10:50:07 +0100 Subject: [PATCH 1/5] Add response body to log --- filedownloader/filedownloader.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/filedownloader/filedownloader.go b/filedownloader/filedownloader.go index d2919dd..49bda5d 100644 --- a/filedownloader/filedownloader.go +++ b/filedownloader/filedownloader.go @@ -108,7 +108,8 @@ func download(context context.Context, client HTTPClient, source string, destina }() if resp.StatusCode != http.StatusOK { - return fmt.Errorf("unable to download file from: %s. Status code: %d", source, resp.StatusCode) + responseBodyBytes, _ := io.ReadAll(resp.Body) + return fmt.Errorf("unable to download file from: %s. Status code: %d. Response body: %s", source, resp.StatusCode, string(responseBodyBytes)) } if _, err = io.Copy(destination, resp.Body); err != nil { From fb6ce6a9c6dcf8e9789a1a6c375531bf12e99e5a Mon Sep 17 00:00:00 2001 From: Tamas Papik Date: Tue, 13 Feb 2024 10:55:03 +0100 Subject: [PATCH 2/5] Update filedownloader.go --- filedownloader/filedownloader.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/filedownloader/filedownloader.go b/filedownloader/filedownloader.go index 49bda5d..e6be4bd 100644 --- a/filedownloader/filedownloader.go +++ b/filedownloader/filedownloader.go @@ -108,7 +108,10 @@ func download(context context.Context, client HTTPClient, source string, destina }() if resp.StatusCode != http.StatusOK { - responseBodyBytes, _ := io.ReadAll(resp.Body) + responseBodyBytes, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("unable to download file from: %s. Status code: %d", source, resp.StatusCode) + } return fmt.Errorf("unable to download file from: %s. Status code: %d. Response body: %s", source, resp.StatusCode, string(responseBodyBytes)) } From 9f9dc8c2bd3539aad879962e4ba092e2f5ac366b Mon Sep 17 00:00:00 2001 From: Tamas Papik Date: Tue, 13 Feb 2024 16:32:40 +0100 Subject: [PATCH 3/5] pribt whole request --- filedownloader/filedownloader.go | 11 +++++++---- filedownloader/filedownloader_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/filedownloader/filedownloader.go b/filedownloader/filedownloader.go index e6be4bd..e02738b 100644 --- a/filedownloader/filedownloader.go +++ b/filedownloader/filedownloader.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "net/http/httputil" "os" "github.com/bitrise-io/go-utils/log" @@ -108,11 +109,13 @@ func download(context context.Context, client HTTPClient, source string, destina }() if resp.StatusCode != http.StatusOK { - responseBodyBytes, err := io.ReadAll(resp.Body) - if err != nil { - return fmt.Errorf("unable to download file from: %s. Status code: %d", source, resp.StatusCode) + if resp.Body != nil { + responseBytes, err := httputil.DumpResponse(resp, true) + if err == nil { + return fmt.Errorf("unable to download file from: %s. Status code: %d. Response: %s", source, resp.StatusCode, string(responseBytes)) + } } - return fmt.Errorf("unable to download file from: %s. Status code: %d. Response body: %s", source, resp.StatusCode, string(responseBodyBytes)) + return fmt.Errorf("unable to download file from: %s. Status code: %d", source, resp.StatusCode) } if _, err = io.Copy(destination, resp.Body); err != nil { diff --git a/filedownloader/filedownloader_test.go b/filedownloader/filedownloader_test.go index 00cf511..85df0bb 100644 --- a/filedownloader/filedownloader_test.go +++ b/filedownloader/filedownloader_test.go @@ -3,6 +3,7 @@ package filedownloader import ( "errors" "fmt" + "io" "io/ioutil" "net/http" "net/http/httptest" @@ -35,7 +36,7 @@ func Test_get_Success(t *testing.T) { assertFileContent(t, path, "filecontent1") } -func Test_get_InvalidStatusCode(t *testing.T) { +func Test_get_InvalidStatusCode_NoBody(t *testing.T) { // Given path := givenTempPath(t) url := "http://url.com" @@ -55,6 +56,28 @@ func Test_get_InvalidStatusCode(t *testing.T) { assertFileNotExists(t, path) } +func Test_get_InvalidStatusCode_WithBody(t *testing.T) { + // Given + path := givenTempPath(t) + url := "http://url.com" + statusCode := 404 + expectedErr := fmt.Errorf("unable to download file from: %s. Status code: %d. Response: HTTP/0.0 404 Not Found\r\ntest-header: test-header-value\r\n\r\ntest-body", url, statusCode) + mockedHTTPClient := givenHTTPClient( + http.Response{ + StatusCode: statusCode, + Header: http.Header{"test-header": []string{"test-header-value"}}, + Body: io.NopCloser(strings.NewReader("test-body")), + }) + downloader := givenFileDownloader(mockedHTTPClient) + + // When + err := downloader.Get(path, url) + + // Then + require.Equal(t, expectedErr, err) + assertFileNotExists(t, path) +} + func Test_get_HTTPError(t *testing.T) { // Given path := givenTempPath(t) From f818c429db832b53d729dd33bf04d4db2fc62789 Mon Sep 17 00:00:00 2001 From: Tamas Papik Date: Wed, 14 Feb 2024 11:04:25 +0100 Subject: [PATCH 4/5] pr change requests --- filedownloader/filedownloader.go | 8 +++----- filedownloader/filedownloader_test.go | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/filedownloader/filedownloader.go b/filedownloader/filedownloader.go index e02738b..731ddfc 100644 --- a/filedownloader/filedownloader.go +++ b/filedownloader/filedownloader.go @@ -109,11 +109,9 @@ func download(context context.Context, client HTTPClient, source string, destina }() if resp.StatusCode != http.StatusOK { - if resp.Body != nil { - responseBytes, err := httputil.DumpResponse(resp, true) - if err == nil { - return fmt.Errorf("unable to download file from: %s. Status code: %d. Response: %s", source, resp.StatusCode, string(responseBytes)) - } + responseBytes, err := httputil.DumpResponse(resp, true) + if err == nil { + return fmt.Errorf("unable to download file from: %s. Status code: %d. Response: %s", source, resp.StatusCode, string(responseBytes)) } return fmt.Errorf("unable to download file from: %s. Status code: %d", source, resp.StatusCode) } diff --git a/filedownloader/filedownloader_test.go b/filedownloader/filedownloader_test.go index 85df0bb..5d2f7af 100644 --- a/filedownloader/filedownloader_test.go +++ b/filedownloader/filedownloader_test.go @@ -41,7 +41,7 @@ func Test_get_InvalidStatusCode_NoBody(t *testing.T) { path := givenTempPath(t) url := "http://url.com" statusCode := 404 - expectedErr := fmt.Errorf("unable to download file from: %s. Status code: %d", url, statusCode) + expectedErr := fmt.Errorf("unable to download file from: %s. Status code: %d. Response: HTTP/0.0 404 Not Found\r\nContent-Length: 0\r\n\r\n", url, statusCode) mockedHTTPClient := givenHTTPClient( http.Response{ StatusCode: statusCode, From a6f2e155906d88792e7197a8cce6842bff376a1b Mon Sep 17 00:00:00 2001 From: Tamas Papik Date: Wed, 14 Feb 2024 14:50:13 +0100 Subject: [PATCH 5/5] changed variables because of the linter --- filedownloader/filedownloader_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/filedownloader/filedownloader_test.go b/filedownloader/filedownloader_test.go index 5d2f7af..a1e7c9a 100644 --- a/filedownloader/filedownloader_test.go +++ b/filedownloader/filedownloader_test.go @@ -41,7 +41,7 @@ func Test_get_InvalidStatusCode_NoBody(t *testing.T) { path := givenTempPath(t) url := "http://url.com" statusCode := 404 - expectedErr := fmt.Errorf("unable to download file from: %s. Status code: %d. Response: HTTP/0.0 404 Not Found\r\nContent-Length: 0\r\n\r\n", url, statusCode) + expectedErrString := fmt.Sprintf("unable to download file from: %s. Status code: %d. Response: HTTP/0.0 404 Not Found\r\nContent-Length: 0\r\n\r\n", url, statusCode) mockedHTTPClient := givenHTTPClient( http.Response{ StatusCode: statusCode, @@ -52,7 +52,7 @@ func Test_get_InvalidStatusCode_NoBody(t *testing.T) { err := downloader.Get(path, url) // Then - require.Equal(t, expectedErr, err) + require.Equal(t, errors.New(expectedErrString).Error(), err.Error()) assertFileNotExists(t, path) }