Skip to content

Commit

Permalink
download: Surface bundle download errors via debug logging
Browse files Browse the repository at this point in the history
This change logs the error response body at debug level.
Since the errors could contain senstive info we don't
include them in the status message. So this approach helps to
get more information about the error at debug log level which
is mostly used in a non-prod setup.

Fixes: #6609

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
  • Loading branch information
ashutosh-narkar committed Mar 27, 2024
1 parent ea0dc02 commit 630b746
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 0 deletions.
7 changes: 7 additions & 0 deletions download/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,13 @@ func (d *Downloader) download(ctx context.Context, m metrics.Metrics) (*download
longPoll: d.longPollingEnabled,
}, nil
default:
if d.logger.GetLevel() == logging.Debug && resp.Body != nil {
body, err := io.ReadAll(resp.Body)
if err == nil {
d.logger.Debug("bundle download error response with response body: %s", body)
}
}

return nil, HTTPError{StatusCode: resp.StatusCode}
}
}
Expand Down
45 changes: 45 additions & 0 deletions download/download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,51 @@ func TestFailureUnexpected(t *testing.T) {
}
}

func TestFailureUnexpectedWithResponseBody(t *testing.T) {

ctx := context.Background()

expResp := "this is a bad http response"

fixture := newTestFixture(t)
fixture.server.expCode = 500
fixture.server.expResp = expResp

defer fixture.server.stop()

d := New(Config{}, fixture.client, "/bundles/test/bundle1")

logger := test.New()
logger.SetLevel(logging.Debug)
d.logger = logger

err := d.oneShot(ctx)
if err == nil {
t.Fatal("expected error")
}
var hErr HTTPError
if !errors.As(err, &hErr) {
t.Fatal("expected HTTPError")
}
if hErr.StatusCode != 500 {
t.Fatal("expected status code 500")
}

expectLogged := fmt.Sprintf("bundle download error response with response body: %s", expResp)

var found bool
for _, entry := range logger.Entries() {
if entry.Message == expectLogged {
found = true
break
}
}

if !found {
t.Errorf("Expected log entry: %s", expectLogged)
}
}

func TestEtagInResponse(t *testing.T) {
ctx := context.Background()
fixture := newTestFixture(t)
Expand Down
6 changes: 6 additions & 0 deletions download/testharness.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ type testServer struct {
t *testing.T
customAuth func(http.ResponseWriter, *http.Request) error
expCode int
expResp string
expEtag string
expAuth string
bundles map[string]bundle.Bundle
Expand Down Expand Up @@ -374,6 +375,11 @@ func (t *testServer) handle(w http.ResponseWriter, r *http.Request) {

if t.expCode != 0 {
w.WriteHeader(t.expCode)

if t.expResp != "" {
w.Write([]byte(t.expResp))
}

return
}

Expand Down

0 comments on commit 630b746

Please sign in to comment.