Skip to content

Commit

Permalink
Remove clienterr.Timeout (not needed)
Browse files Browse the repository at this point in the history
- add generic HTTP response error
- set User-Agent header
  • Loading branch information
barrettj12 committed Aug 18, 2023
1 parent c020800 commit 71ce438
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 37 deletions.
36 changes: 29 additions & 7 deletions internals/overlord/logstate/clienterr/clienterr.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ Errors in this package should be pattern-matched using errors.As:
package clienterr

import (
"bytes"
"fmt"
"io"
"net/http"
"time"
)

Expand All @@ -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
}
6 changes: 0 additions & 6 deletions internals/overlord/logstate/gatherer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
24 changes: 4 additions & 20 deletions internals/overlord/logstate/loki/loki.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 2 additions & 4 deletions internals/overlord/logstate/loki/loki_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}()

Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 71ce438

Please sign in to comment.