Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrap doRequest calls for error handling #11158

Merged
merged 3 commits into from
Sep 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
kisunji marked this conversation as resolved.
Show resolved Hide resolved
// 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
acpana marked this conversation as resolved.
Show resolved Hide resolved
// 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)
acpana marked this conversation as resolved.
Show resolved Hide resolved

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