diff --git a/internals/overlord/logstate/clienterr/clienterr.go b/internals/overlord/logstate/clienterr/clienterr.go index be76b6e25..c756ae136 100644 --- a/internals/overlord/logstate/clienterr/clienterr.go +++ b/internals/overlord/logstate/clienterr/clienterr.go @@ -28,6 +28,10 @@ Errors in this package should be pattern-matched using errors.As: package clienterr import ( + "bytes" + "fmt" + "io" + "net/http" "time" ) @@ -45,15 +49,33 @@ func (e *Backoff) Error() string { return errStr } -// Timeout should be returned if the client's Flush times out. -type Timeout struct { - Err error +// ErrorResponse represents an HTTP error response from the server +// (4xx or 5xx). +type ErrorResponse struct { + StatusCode int + Body bytes.Buffer + ReadErr error } -func (e *Timeout) Error() string { - return "client flush timed out: " + e.Err.Error() +func (e *ErrorResponse) Error() string { + errStr := fmt.Sprintf("server returned HTTP %d\n", e.StatusCode) + if e.Body.Len() > 0 { + errStr += fmt.Sprintf(`response body: +%s +`, e.Body.String()) + } + if e.ReadErr != nil { + errStr += "cannot read response body: " + e.ReadErr.Error() + } + return errStr } -func (e *Timeout) Unwrap() error { - return e.Err +// ErrorFromResponse generates a *ErrorResponse from a failed *http.Response. +// NB: this function reads the response body. +func ErrorFromResponse(resp *http.Response) *ErrorResponse { + err := &ErrorResponse{} + err.StatusCode = resp.StatusCode + _, readErr := io.CopyN(&err.Body, resp.Body, 1024) + err.ReadErr = readErr + return err } diff --git a/internals/overlord/logstate/gatherer.go b/internals/overlord/logstate/gatherer.go index e674cd70e..d290471f8 100644 --- a/internals/overlord/logstate/gatherer.go +++ b/internals/overlord/logstate/gatherer.go @@ -214,12 +214,6 @@ func (g *logGatherer) handleClientErr(err error) { return } - timeout := &clienterr.Timeout{} - if errors.As(err, &timeout) { - logger.Noticef("Target %q: %v", g.targetName, err) - return - } - logger.Noticef("Cannot flush logs to target %q: %v", g.targetName, err) } diff --git a/internals/overlord/logstate/loki/loki.go b/internals/overlord/logstate/loki/loki.go index db09cd105..885167309 100644 --- a/internals/overlord/logstate/loki/loki.go +++ b/internals/overlord/logstate/loki/loki.go @@ -18,16 +18,15 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" - "net" "net/http" "sort" "strconv" "strings" "time" + "github.com/canonical/pebble/cmd" "github.com/canonical/pebble/internals/overlord/logstate/clienterr" "github.com/canonical/pebble/internals/plan" "github.com/canonical/pebble/internals/servicelog" @@ -98,12 +97,11 @@ func (c *Client) Flush(ctx context.Context) error { return fmt.Errorf("creating HTTP request: %v", err) } httpReq.Header.Set("Content-Type", "application/json; charset=utf-8") - //httpReq.Header.Set("User-Agent", ???) TODO - //httpReq.Header.Set("Accept", ???) TODO + httpReq.Header.Set("User-Agent", fmt.Sprintf("pebble/%s", cmd.Version)) resp, err := c.httpClient.Do(httpReq) if err != nil { - return handleHTTPClientErr(err) + return err } return c.handleServerResponse(resp) } @@ -151,15 +149,6 @@ type stream struct { type lokiEntry [2]string -func handleHTTPClientErr(err error) error { - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - return &clienterr.Timeout{Err: err} - } - - return err -} - func (c *Client) handleServerResponse(resp *http.Response) error { defer func() { // Drain request body to allow connection reuse @@ -181,12 +170,7 @@ func (c *Client) handleServerResponse(resp *http.Response) error { case code >= 400: // Request to Loki failed - b, err := io.ReadAll(io.LimitReader(resp.Body, 1024)) - if err != nil { - b = append(b, []byte("//couldn't read response body: ")...) - b = append(b, []byte(err.Error())...) - } - return fmt.Errorf("cannot send logs to Loki (HTTP %d), response body:\n%s", resp.StatusCode, b) + return clienterr.ErrorFromResponse(resp) } return nil diff --git a/internals/overlord/logstate/loki/loki_test.go b/internals/overlord/logstate/loki/loki_test.go index e146e66c4..29e9ca92b 100644 --- a/internals/overlord/logstate/loki/loki_test.go +++ b/internals/overlord/logstate/loki/loki_test.go @@ -172,8 +172,7 @@ func (*suite) TestFlushCancelContext(c *C) { defer cancel() err := client.Flush(ctx) - timeout := &clienterr.Timeout{} - c.Assert(errors.As(err, &timeout), Equals, true) + c.Assert(err, ErrorMatches, ".*context deadline exceeded.*") close(flushReturned) }() @@ -203,8 +202,7 @@ func (*suite) TestServerTimeout(c *C) { c.Assert(err, IsNil) err = client.Flush(context.Background()) - timeout := &clienterr.Timeout{} - c.Assert(errors.As(err, &timeout), Equals, true) + c.Assert(err, ErrorMatches, ".*context deadline exceeded.*") } func (*suite) TestTooManyRequests(c *C) {