Skip to content

Commit

Permalink
wrap few doRequest calls for error handling (#11158)
Browse files Browse the repository at this point in the history
  • Loading branch information
FFMMM authored Sep 29, 2021
1 parent 1904058 commit 8bb6d85
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 38 deletions.
27 changes: 16 additions & 11 deletions api/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package api

import (
"bufio"
"bytes"
"context"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -602,6 +602,8 @@ func (a *Agent) AgentHealthServiceByIDOpts(serviceID string, q *QueryOptions) (s
r.setQueryOptions(q)
r.params.Add("format", "json")
r.header.Set("Accept", "application/json")
// not a lot of value in wrapping the doRequest call in a requireHttpCodes call
// we manipulate the resp body and the require calls "swallow" the content on err
_, resp, err := a.c.doRequest(r)
if err != nil {
return "", nil, err
Expand Down Expand Up @@ -641,6 +643,8 @@ func (a *Agent) AgentHealthServiceByNameOpts(service string, q *QueryOptions) (s
r.setQueryOptions(q)
r.params.Add("format", "json")
r.header.Set("Accept", "application/json")
// not a lot of value in wrapping the doRequest call in a requireHttpCodes call
// we manipulate the resp body and the require calls "swallow" the content on err
_, resp, err := a.c.doRequest(r)
if err != nil {
return "", nil, err
Expand Down Expand Up @@ -1233,19 +1237,20 @@ func (a *Agent) updateTokenOnce(target, token string, q *WriteOptions) (*WriteMe
r.setWriteOptions(q)
r.obj = &AgentToken{Token: token}

rtt, resp, err := a.c.doRequest(r)
if err != nil {
return nil, 0, err
}
defer closeResponseBody(resp)

rtt, resp, err := requireOK(a.c.doRequest(r))
wm := &WriteMeta{RequestTime: rtt}

if resp.StatusCode != 200 {
var buf bytes.Buffer
io.Copy(&buf, resp.Body)
return wm, resp.StatusCode, fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, buf.Bytes())
if err != nil {
// if the error was bc of a non 200 response
// from requireOK
var statusE StatusError
if errors.As(err, &statusE) {
return wm, statusE.Code, statusE
}
// otherwise, the error came via doRequest
return nil, 500, err
}
defer closeResponseBody(resp)

return wm, resp.StatusCode, nil
}
29 changes: 5 additions & 24 deletions api/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,12 @@ func (c *Client) Debug() *Debug {
// Heap returns a pprof heap dump
func (d *Debug) Heap() ([]byte, error) {
r := d.c.newRequest("GET", "/debug/pprof/heap")
_, resp, err := d.c.doRequest(r)
_, resp, err := requireOK(d.c.doRequest(r))
if err != nil {
return nil, fmt.Errorf("error making request: %s", err)
}
defer closeResponseBody(resp)

if resp.StatusCode != 200 {
return nil, generateUnexpectedResponseCodeError(resp)
}

// We return a raw response because we're just passing through a response
// from the pprof handlers
body, err := ioutil.ReadAll(resp.Body)
Expand All @@ -52,16 +48,12 @@ func (d *Debug) Profile(seconds int) ([]byte, error) {
// Capture a profile for the specified number of seconds
r.params.Set("seconds", strconv.Itoa(seconds))

_, resp, err := d.c.doRequest(r)
_, resp, err := requireOK(d.c.doRequest(r))
if err != nil {
return nil, fmt.Errorf("error making request: %s", err)
}
defer closeResponseBody(resp)

if resp.StatusCode != 200 {
return nil, generateUnexpectedResponseCodeError(resp)
}

// We return a raw response because we're just passing through a response
// from the pprof handlers
body, err := ioutil.ReadAll(resp.Body)
Expand All @@ -81,14 +73,11 @@ func (d *Debug) PProf(ctx context.Context, name string, seconds int) (io.ReadClo
// Capture a profile for the specified number of seconds
r.params.Set("seconds", strconv.Itoa(seconds))

_, resp, err := d.c.doRequest(r)
_, resp, err := requireOK(d.c.doRequest(r))
if err != nil {
return nil, fmt.Errorf("error making request: %s", err)
}

if resp.StatusCode != 200 {
return nil, generateUnexpectedResponseCodeError(resp)
}
return resp.Body, nil
}

Expand All @@ -99,16 +88,12 @@ func (d *Debug) Trace(seconds int) ([]byte, error) {
// Capture a trace for the specified number of seconds
r.params.Set("seconds", strconv.Itoa(seconds))

_, resp, err := d.c.doRequest(r)
_, resp, err := requireOK(d.c.doRequest(r))
if err != nil {
return nil, fmt.Errorf("error making request: %s", err)
}
defer closeResponseBody(resp)

if resp.StatusCode != 200 {
return nil, generateUnexpectedResponseCodeError(resp)
}

// We return a raw response because we're just passing through a response
// from the pprof handlers
body, err := ioutil.ReadAll(resp.Body)
Expand All @@ -123,16 +108,12 @@ func (d *Debug) Trace(seconds int) ([]byte, error) {
func (d *Debug) Goroutine() ([]byte, error) {
r := d.c.newRequest("GET", "/debug/pprof/goroutine")

_, resp, err := d.c.doRequest(r)
_, resp, err := requireOK(d.c.doRequest(r))
if err != nil {
return nil, fmt.Errorf("error making request: %s", err)
}
defer closeResponseBody(resp)

if resp.StatusCode != 200 {
return nil, generateUnexpectedResponseCodeError(resp)
}

// We return a raw response because we're just passing through a response
// from the pprof handlers
body, err := ioutil.ReadAll(resp.Body)
Expand Down
6 changes: 3 additions & 3 deletions api/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ func (k *KV) getInternal(key string, params map[string]string, q *QueryOptions)
r.params.Set(param, val)
}
rtt, resp, err := k.c.doRequest(r)
rtt, resp, err = requireHttpCodes(rtt, resp, err, 200, 404)

if err != nil {
return nil, nil, err
}
Expand All @@ -140,10 +142,8 @@ func (k *KV) getInternal(key string, params map[string]string, q *QueryOptions)
if resp.StatusCode == 404 {
closeResponseBody(resp)
return nil, qm, nil
} else if resp.StatusCode != 200 {
closeResponseBody(resp)
return nil, nil, fmt.Errorf("Unexpected response code: %d", resp.StatusCode)
}

return resp, qm, nil
}

Expand Down

0 comments on commit 8bb6d85

Please sign in to comment.