[api] Return a shapely error for unexpected response (#16743)

* Add UnexpectedResultError to nomad/api

This allows users to perform additional status-based behavior by rehydrating the error using `errors.As` inside of consumers.

.changelog/16743.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+api: return a structured error for unexpected responses
+```

api/api.go

@@ -895,13 +895,16 @@ func (c *Client) websocket(endpoint string, q *QueryOptions) (*websocket.Conn, *
 	conn, resp, err := dialer.Dial(rhttp.URL.String(), rhttp.Header)
 
 	// check resp status code, as it's more informative than handshake error we get from ws library
-	if resp != nil && resp.StatusCode != 101 {
+	if resp != nil && resp.StatusCode != http.StatusSwitchingProtocols {
 		var buf bytes.Buffer
 
 		if resp.Header.Get("Content-Encoding") == "gzip" {
 			greader, err := gzip.NewReader(resp.Body)
 			if err != nil {
-				return nil, nil, fmt.Errorf("Unexpected response code: %d", resp.StatusCode)
+				return nil, nil, newUnexpectedResponseError(
+					fromStatusCode(resp.StatusCode),
+					withExpectedStatuses([]int{http.StatusSwitchingProtocols}),
+					withError(err))
 			}
 			io.Copy(&buf, greader)
 		} else {
@@ -909,7 +912,11 @@ func (c *Client) websocket(endpoint string, q *QueryOptions) (*websocket.Conn, *
 		}
 		resp.Body.Close()
 
-		return nil, nil, fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, buf.Bytes())
+		return nil, nil, newUnexpectedResponseError(
+			fromStatusCode(resp.StatusCode),
+			withExpectedStatuses([]int{http.StatusSwitchingProtocols}),
+			withBody(fmt.Sprint(buf.Bytes())),
+		)
 	}
 
 	return conn, resp, err
@@ -1129,24 +1136,6 @@ func encodeBody(obj interface{}) (io.Reader, error) {
 	return buf, nil
 }
 
-// requireOK is used to wrap doRequest and check for a 200
-func requireOK(d time.Duration, resp *http.Response, e error) (time.Duration, *http.Response, error) {
-	if e != nil {
-		if resp != nil {
-			resp.Body.Close()
-		}
-		return d, nil, e
-	}
-	if resp.StatusCode != 200 {
-		var buf bytes.Buffer
-		_, _ = io.Copy(&buf, resp.Body)
-		_ = resp.Body.Close()
-		body := strings.TrimSpace(buf.String())
-		return d, nil, fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, body)
-	}
-	return d, resp, nil
-}
-
 // Context returns the context used for canceling HTTP requests related to this query
 func (o *QueryOptions) Context() context.Context {
 	if o != nil && o.ctx != nil {

api/error_unexpected_response.go

@@ -0,0 +1,175 @@
+package api
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"net/http"
+	"strings"
+	"time"
+
+	"golang.org/x/exp/slices"
+)
+
+// UnexpectedResponseError tracks the components for API errors encountered when
+// requireOK and requireStatusIn's conditions are not met.
+type UnexpectedResponseError struct {
+	expected   []int
+	statusCode int
+	statusText string
+	body       string
+	err        error
+	additional error
+}
+
+func (e UnexpectedResponseError) HasExpectedStatuses() bool { return len(e.expected) > 0 }
+func (e UnexpectedResponseError) ExpectedStatuses() []int   { return e.expected }
+func (e UnexpectedResponseError) HasStatusCode() bool       { return e.statusCode != 0 }
+func (e UnexpectedResponseError) StatusCode() int           { return e.statusCode }
+func (e UnexpectedResponseError) HasStatusText() bool       { return e.statusText != "" }
+func (e UnexpectedResponseError) StatusText() string        { return e.statusText }
+func (e UnexpectedResponseError) HasBody() bool             { return e.body != "" }
+func (e UnexpectedResponseError) Body() string              { return e.body }
+func (e UnexpectedResponseError) HasError() bool            { return e.err != nil }
+func (e UnexpectedResponseError) Unwrap() error             { return e.err }
+func (e UnexpectedResponseError) HasAdditional() bool       { return e.additional != nil }
+func (e UnexpectedResponseError) Additional() error         { return e.additional }
+func newUnexpectedResponseError(src unexpectedResponseErrorSource, opts ...unexpectedResponseErrorOption) UnexpectedResponseError {
+	nErr := src()
+	for _, opt := range opts {
+		opt(nErr)
+	}
+	if nErr.statusText == "" {
+		// the stdlib's http.StatusText function is a good place to start
+		nErr.statusFromCode(http.StatusText)
+	}
+
+	return *nErr
+}
+
+// Use textual representation of the given integer code. Called when status text
+// is not set using the WithStatusText option.
+func (e UnexpectedResponseError) statusFromCode(f func(int) string) {
+	e.statusText = f(e.statusCode)
+	if !e.HasStatusText() {
+		e.statusText = "unknown status code"
+	}
+}
+
+func (e UnexpectedResponseError) Error() string {
+	var eTxt strings.Builder
+	eTxt.WriteString("Unexpected response code")
+	if e.HasBody() || e.HasStatusCode() {
+		eTxt.WriteString(": ")
+	}
+	if e.HasStatusCode() {
+		eTxt.WriteString(fmt.Sprint(e.statusCode))
+		if e.HasBody() {
+			eTxt.WriteRune(' ')
+		}
+	}
+	if e.HasBody() {
+		eTxt.WriteString(fmt.Sprintf("(%s)", e.body))
+	}
+
+	if e.HasAdditional() {
+		eTxt.WriteString(fmt.Sprintf(". Additionally, an error occurred while constructing this error (%s); the body might be truncated or missing.", e.additional.Error()))
+	}
+
+	return eTxt.String()
+}
+
+// UnexpectedResponseErrorOptions are functions passed to NewUnexpectedResponseError
+// to customize the created error.
+type unexpectedResponseErrorOption func(*UnexpectedResponseError)
+
+// withError allows the addition of a Go error that may have been encountered
+// while processing the response. For example, if there is an error constructing
+// the gzip reader to process a gzip-encoded response body.
+func withError(e error) unexpectedResponseErrorOption {
+	return func(u *UnexpectedResponseError) { u.err = e }
+}
+
+// withBody overwrites the Body value with the provided custom value
+func withBody(b string) unexpectedResponseErrorOption {
+	return func(u *UnexpectedResponseError) { u.body = b }
+}
+
+// withStatusText overwrites the StatusText value the provided custom value
+func withStatusText(st string) unexpectedResponseErrorOption {
+	return func(u *UnexpectedResponseError) { u.statusText = st }
+}
+
+// withExpectedStatuses provides a list of statuses that the receiving function
+// expected to receive. This can be used by API callers to provide more feedback
+// to end-users.
+func withExpectedStatuses(s []int) unexpectedResponseErrorOption {
+	return func(u *UnexpectedResponseError) { u.expected = slices.Clone(s) }
+}
+
+// unexpectedResponseErrorSource provides the basis for a NewUnexpectedResponseError.
+type unexpectedResponseErrorSource func() *UnexpectedResponseError
+
+// fromHTTPResponse read an open HTTP response, drains and closes its body as
+// the data for the UnexpectedResponseError.
+func fromHTTPResponse(resp *http.Response) unexpectedResponseErrorSource {
+	return func() *UnexpectedResponseError {
+		u := new(UnexpectedResponseError)
+
+		if resp != nil {
+			// collect and close the body
+			var buf bytes.Buffer
+			if _, e := io.Copy(&buf, resp.Body); e != nil {
+				u.additional = e
+			}
+
+			// Body has been tested as safe to close more than once
+			_ = resp.Body.Close()
+			body := strings.TrimSpace(buf.String())
+
+			// make and return the error
+			u.statusCode = resp.StatusCode
+			u.statusText = strings.TrimSpace(strings.TrimPrefix(resp.Status, fmt.Sprint(resp.StatusCode)))
+			u.body = body
+		}
+		return u
+	}
+}
+
+// fromStatusCode attempts to resolve the status code to status text using
+// the resolving function provided inside of the NewUnexpectedResponseError
+// implementation.
+func fromStatusCode(sc int) unexpectedResponseErrorSource {
+	return func() *UnexpectedResponseError { return &UnexpectedResponseError{statusCode: sc} }
+}
+
+// doRequestWrapper is a function that wraps the client's doRequest method
+// and can be used to provide error and response handling
+type doRequestWrapper = func(time.Duration, *http.Response, error) (time.Duration, *http.Response, error)
+
+// requireOK is used to wrap doRequest and check for a 200
+func requireOK(d time.Duration, resp *http.Response, e error) (time.Duration, *http.Response, error) {
+	f := requireStatusIn(http.StatusOK)
+	return f(d, resp, e)
+}
+
+// requireStatusIn is a doRequestWrapper generator that takes expected HTTP
+// response codes and validates that the received response code is among them
+func requireStatusIn(statuses ...int) doRequestWrapper {
+	return func(d time.Duration, resp *http.Response, e error) (time.Duration, *http.Response, error) {
+		if e != nil {
+			if resp != nil {
+				_ = resp.Body.Close()
+			}
+			return d, nil, e
+		}
+
+		for _, status := range statuses {
+			if resp.StatusCode == status {
+				return d, resp, nil
+			}
+		}
+
+		return d, nil, newUnexpectedResponseError(fromHTTPResponse(resp), withExpectedStatuses(statuses))
+	}
+}

api/error_unexpected_response_test.go

@@ -0,0 +1,266 @@
+package api_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"net/url"
+	"strings"
+	"testing"
+	"testing/iotest"
+	"time"
+
+	"github.com/felixge/httpsnoop"
+	"github.com/hashicorp/nomad/api"
+	"github.com/hashicorp/nomad/api/internal/testutil"
+	"github.com/shoenig/test/must"
+)
+
+const mockNamespaceBody = `{"Capabilities":null,"CreateIndex":1,"Description":"Default shared namespace","Hash":"C7UbjDwBK0dK8wQq7Izg7SJIzaV+lIo2X7wRtzY3pSw=","Meta":null,"ModifyIndex":1,"Name":"default","Quota":""}`
+
+func TestUnexpectedResponseError(t *testing.T) {
+	testutil.Parallel(t)
+	a := mockserver(t)
+	cfg := api.DefaultConfig()
+	cfg.Address = a
+
+	c, e := api.NewClient(cfg)
+	must.NoError(t, e)
+
+	type testCase struct {
+		testFunc   func()
+		statusCode *int
+		body       *int
+	}
+
+	// ValidateServer ensures that the mock server handles the default namespace
+	// correctly. This ensures that the routing rule for this path is at least
+	// correct and that the mock server is passing its address to the client
+	// properly.
+	t.Run("ValidateServer", func(t *testing.T) {
+		n, _, err := c.Namespaces().Info("default", nil)
+		must.NoError(t, err)
+		var ns api.Namespace
+		err = unmock(t, mockNamespaceBody, &ns)
+		must.NoError(t, err)
+		must.Eq(t, ns, *n)
+	})
+
+	// WrongStatus tests that an UnexpectedResponseError is generated and filled
+	// with the correct data when a response code that the API client wasn't
+	// looking for is returned by the server.
+	t.Run("WrongStatus", func(t *testing.T) {
+		testutil.Parallel(t)
+		n, _, err := c.Namespaces().Info("badStatus", nil)
+		must.Nil(t, n)
+		must.Error(t, err)
+		t.Logf("err: %v", err)
+
+		ure, ok := err.(api.UnexpectedResponseError)
+		must.True(t, ok)
+
+		must.True(t, ure.HasStatusCode())
+		must.Eq(t, http.StatusAccepted, ure.StatusCode())
+
+		must.True(t, ure.HasStatusText())
+		must.Eq(t, http.StatusText(http.StatusAccepted), ure.StatusText())
+
+		must.True(t, ure.HasBody())
+		must.Eq(t, mockNamespaceBody, ure.Body())
+	})
+
+	// NotFound tests that an UnexpectedResponseError is generated and filled
+	// with the correct data when a `404 Not Found`` is returned to the API
+	// client, since the requireOK wrapper doesn't "expect" 404s.
+	t.Run("NotFound", func(t *testing.T) {
+		testutil.Parallel(t)
+		n, _, err := c.Namespaces().Info("wat", nil)
+		must.Nil(t, n)
+		must.Error(t, err)
+		t.Logf("err: %v", err)
+
+		ure, ok := err.(api.UnexpectedResponseError)
+		must.True(t, ok)
+
+		must.True(t, ure.HasStatusCode())
+		must.Eq(t, http.StatusNotFound, ure.StatusCode())
+
+		must.True(t, ure.HasStatusText())
+		must.Eq(t, http.StatusText(http.StatusNotFound), ure.StatusText())
+
+		must.True(t, ure.HasBody())
+		must.Eq(t, "Namespace not found", ure.Body())
+	})
+
+	// EarlyClose tests what happens when an error occurs during the building of
+	// the UnexpectedResponseError using FromHTTPRequest.
+	t.Run("EarlyClose", func(t *testing.T) {
+		testutil.Parallel(t)
+		n, _, err := c.Namespaces().Info("earlyClose", nil)
+		must.Nil(t, n)
+		must.Error(t, err)
+
+		t.Logf("e: %v\n", err)
+		ure, ok := err.(api.UnexpectedResponseError)
+		must.True(t, ok)
+
+		must.True(t, ure.HasStatusCode())
+		must.Eq(t, http.StatusInternalServerError, ure.StatusCode())
+
+		must.True(t, ure.HasStatusText())
+		must.Eq(t, http.StatusText(http.StatusInternalServerError), ure.StatusText())
+
+		must.True(t, ure.HasAdditional())
+		must.ErrorContains(t, err, "the body might be truncated")
+
+		must.True(t, ure.HasBody())
+		must.Eq(t, "{", ure.Body()) // The body is truncated to the first byte
+	})
+}
+
+// mockserver creates a httptest.Server that can be used to serve simple mock
+// data, which is faster than starting a real Nomad agent.
+func mockserver(t *testing.T) string {
+	port := testutil.PortAllocator.One()
+
+	mux := http.NewServeMux()
+	mux.Handle("/v1/namespace/earlyClose", closingHandler(http.StatusInternalServerError, mockNamespaceBody))
+	mux.Handle("/v1/namespace/badStatus", testHandler(http.StatusAccepted, mockNamespaceBody))
+	mux.Handle("/v1/namespace/default", testHandler(http.StatusOK, mockNamespaceBody))
+	mux.Handle("/v1/namespace/", testNotFoundHandler("Namespace not found"))
+	mux.Handle("/v1/namespace", http.NotFoundHandler())
+	mux.Handle("/v1", http.NotFoundHandler())
+	mux.Handle("/", testHandler(http.StatusOK, "ok"))
+
+	lMux := testLogRequestHandler(t, mux)
+	ts := httptest.NewUnstartedServer(lMux)
+	ts.Config.Addr = fmt.Sprintf("127.0.0.1:%d", port)
+
+	t.Logf("starting mock server on %s", ts.Config.Addr)
+	ts.Start()
+	t.Cleanup(func() {
+		t.Log("stopping mock server")
+		ts.Close()
+	})
+
+	// Test the server
+	tc := ts.Client()
+	resp, err := tc.Get(func() string { p, _ := url.JoinPath(ts.URL, "/"); return p }())
+	must.NoError(t, err)
+	defer resp.Body.Close()
+	b, err := io.ReadAll(resp.Body)
+	must.NoError(t, err)
+	t.Logf("checking mock server, got resp: %s", b)
+
+	// If we get here, the mock server is running and ready for requests.
+	return ts.URL
+}
+
+// addMockHeaders sets the common Nomad headers to values sufficient to be
+// parsed into api.QueryMeta
+func addMockHeaders(h http.Header) {
+	h.Add("X-Nomad-Knownleader", "true")
+	h.Add("X-Nomad-Lastcontact", "0")
+	h.Add("X-Nomad-Index", "1")
+	h.Add("Content-Type", "application/json")
+}
+
+// testNotFoundHandler creates a testHandler preconfigured with status code 404.
+func testNotFoundHandler(b string) http.Handler { return testHandler(http.StatusNotFound, b) }
+
+// testNotFoundHandler creates a testHandler preconfigured with status code 200.
+func testOKHandler(b string) http.Handler { return testHandler(http.StatusOK, b) }
+
+// testHandler is a helper function that writes a Nomad-like server response
+// with the necessary headers to make the API client happy
+func testHandler(sc int, b string) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		addMockHeaders(w.Header())
+		w.WriteHeader(sc)
+		w.Write([]byte(b))
+	})
+}
+
+// closingHandler is a handler that terminates the response body early in the
+// reading process
+func closingHandler(sc int, b string) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+
+		// We need a misbehaving reader to test network effects when collecting
+		// the http.Response data into a UnexpectedResponseError
+		er := iotest.TimeoutReader( // TimeoutReader throws an error on the second read
+			iotest.OneByteReader( // OneByteReader yields a byte at a time, causing multiple reads
+				strings.NewReader(mockNamespaceBody),
+			),
+		)
+
+		// We need to set content-length to the true value it _should_ be so the
+		// API-side reader knows it's a short read.
+		w.Header().Set("content-length", fmt.Sprint(len(mockNamespaceBody)))
+		addMockHeaders(w.Header())
+		w.WriteHeader(sc)
+
+		// Using io.Copy to send the data into w prevents golang from setting the
+		// content-length itself.
+		io.Copy(w, er)
+	})
+}
+
+// testLogRequestHandler wraps a http.Handler with a logger that writes to the
+// test log output
+func testLogRequestHandler(t *testing.T, h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// call the original http.Handler wrapped in a httpsnoop
+		m := httpsnoop.CaptureMetrics(h, w, r)
+		ri := httpReqInfo{
+			uri:       r.URL.String(),
+			method:    r.Method,
+			ipaddr:    ipAddrFromRemoteAddr(r.RemoteAddr),
+			code:      m.Code,
+			duration:  m.Duration,
+			size:      m.Written,
+			userAgent: r.UserAgent(),
+		}
+		t.Logf(ri.String())
+	})
+}
+
+// httpReqInfo holds all the information used to log a request to the mock server
+type httpReqInfo struct {
+	method    string
+	uri       string
+	referer   string
+	ipaddr    string
+	code      int
+	size      int64
+	duration  time.Duration
+	userAgent string
+}
+
+func (i httpReqInfo) String() string {
+	return fmt.Sprintf(
+		"method=%q uri=%q referer=%q ipaddr=%q code=%d size=%d duration=%q userAgent=%q",
+		i.method, i.uri, i.referer, i.ipaddr, i.code, i.size, i.duration, i.userAgent,
+	)
+}
+
+// ipAddrFromRemoteAddr removes the port from the address:port in remote addr
+// in case of a parse error, the original value is returned unparsed
+func ipAddrFromRemoteAddr(s string) string {
+	if ap, err := netip.ParseAddrPort(s); err == nil {
+		return ap.Addr().String()
+	}
+	return s
+}
+
+// unmock attempts to unmarshal a given mock json body into dst, which should
+// be a pointer to the correct API struct.
+func unmock(t *testing.T, src string, dst any) error {
+	if err := json.Unmarshal([]byte(src), dst); err != nil {
+		return fmt.Errorf("error unmarshaling mock: %w", err)
+	}
+	return nil
+}

api/go.mod

@@ -4,6 +4,7 @@ go 1.20
 
 require (
 	github.com/docker/go-units v0.5.0
+	github.com/felixge/httpsnoop v1.0.3
 	github.com/gorilla/websocket v1.5.0
 	github.com/hashicorp/cronexpr v1.1.1
 	github.com/hashicorp/go-cleanhttp v0.5.2

api/go.sum

@@ -3,6 +3,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4=
 github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
+github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
+github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=

api/internal/testutil/ports.go

@@ -0,0 +1,23 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package testutil
+
+import (
+	"fmt"
+
+	"github.com/shoenig/test/portal"
+)
+
+type fatalTester struct{}
+
+func (t *fatalTester) Fatalf(msg string, args ...any) {
+	panic(fmt.Sprintf(msg, args...))
+}
+
+// PortAllocator is used to acquire unused ports for testing real network
+// listeners.
+var PortAllocator = portal.New(
+	new(fatalTester),
+	portal.WithAddress("127.0.0.1"),
+)

api/internal/testutil/server.go

@@ -28,7 +28,6 @@ import (
 	"github.com/hashicorp/nomad/api/internal/testutil/discover"
 	testing "github.com/mitchellh/go-testing-interface"
 	"github.com/shoenig/test/must"
-	"github.com/shoenig/test/portal"
 	"github.com/shoenig/test/wait"
 )
 
@@ -107,8 +106,7 @@ type ServerConfigCallback func(c *TestServerConfig)
 // defaultServerConfig returns a new TestServerConfig struct pre-populated with
 // usable config for running as server.
 func defaultServerConfig(t testing.T) *TestServerConfig {
-	grabber := portal.New(t)
-	ports := grabber.Grab(3)
+	ports := PortAllocator.Grab(3)
 
 	logLevel := "ERROR"
 	if envLogLevel := os.Getenv("NOMAD_TEST_LOG_LEVEL"); envLogLevel != "" {

api/namespace_test.go

@@ -4,6 +4,8 @@
 package api
 
 import (
+	"errors"
+	"net/http"
 	"testing"
 
 	"github.com/hashicorp/nomad/api/internal/testutil"
@@ -145,3 +147,18 @@ func TestNamespaces_List(t *testing.T) {
 	must.Len(t, 1, resp)
 	must.Eq(t, ns2.Name, resp[0].Name)
 }
+
+func TestNamespace_NotFound(t *testing.T) {
+	testutil.Parallel(t)
+
+	c, s := makeClient(t, nil, nil)
+	defer s.Stop()
+	namespaces := c.Namespaces()
+
+	var ure UnexpectedResponseError
+	_, _, e := namespaces.Info("dummy", nil)
+
+	ok := errors.As(e, &ure)
+	must.True(t, ok)
+	must.Eq(t, http.StatusNotFound, ure.StatusCode())
+}

api/operator.go

@@ -6,8 +6,8 @@ package api
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
+	"net/http"
 	"strconv"
 	"strings"
 	"time"
@@ -341,13 +341,15 @@ func (op *Operator) LicenseGet(q *QueryOptions) (*LicenseReply, *QueryMeta, erro
 	}
 	defer resp.Body.Close()
 
-	if resp.StatusCode == 204 {
+	if resp.StatusCode == http.StatusNoContent {
 		return nil, nil, errors.New("Nomad Enterprise only endpoint")
 	}
 
-	if resp.StatusCode != 200 {
-		body, _ := io.ReadAll(resp.Body)
-		return nil, nil, fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, body)
+	if resp.StatusCode != http.StatusOK {
+		return nil, nil, newUnexpectedResponseError(
+			fromHTTPResponse(resp),
+			withExpectedStatuses([]int{http.StatusOK, http.StatusNoContent}),
+		)
 	}
 
 	err = json.NewDecoder(resp.Body).Decode(&reply)

api/variables.go

@@ -4,14 +4,11 @@
 package api
 
 import (
-	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"net/http"
 	"strings"
-	"time"
 )
 
 const (
@@ -457,39 +454,3 @@ type ErrCASConflict struct {
 func (e ErrCASConflict) Error() string {
 	return fmt.Sprintf("cas conflict: expected ModifyIndex %v; found %v", e.CheckIndex, e.Conflict.ModifyIndex)
 }
-
-// doRequestWrapper is a function that wraps the client's doRequest method
-// and can be used to provide error and response handling
-type doRequestWrapper = func(time.Duration, *http.Response, error) (time.Duration, *http.Response, error)
-
-// requireStatusIn is a doRequestWrapper generator that takes expected HTTP
-// response codes and validates that the received response code is among them
-func requireStatusIn(statuses ...int) doRequestWrapper {
-	fn := func(d time.Duration, resp *http.Response, e error) (time.Duration, *http.Response, error) {
-		if e != nil {
-			if resp != nil {
-				_ = resp.Body.Close()
-			}
-			return d, nil, e
-		}
-
-		for _, status := range statuses {
-			if resp.StatusCode == status {
-				return d, resp, nil
-			}
-		}
-
-		return d, nil, generateUnexpectedResponseCodeError(resp)
-	}
-	return fn
-}
-
-// generateUnexpectedResponseCodeError creates a standardized error
-// when the the API client's newRequest method receives an unexpected
-// HTTP response code when accessing the variable's HTTP API
-func generateUnexpectedResponseCodeError(resp *http.Response) error {
-	var buf bytes.Buffer
-	_, _ = io.Copy(&buf, resp.Body)
-	_ = resp.Body.Close()
-	return fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, buf.Bytes())
-}

go.mod

@@ -188,7 +188,7 @@ require (
 	github.com/docker/go-metrics v0.0.1 // indirect
 	github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect
 	github.com/fatih/color v1.13.0
-	github.com/felixge/httpsnoop v1.0.1 // indirect
+	github.com/felixge/httpsnoop v1.0.3 // indirect
 	github.com/go-ole/go-ole v1.2.6 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect

go.sum

@@ -574,8 +574,9 @@ github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL
 github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
 github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
 github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
-github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ=
 github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
+github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
+github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k=
 github.com/frankban/quicktest v1.10.0/go.mod h1:ui7WezCLWMWxVWr1GETZY3smRy0G4KWq9vcPtJmFl7Y=
 github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM0I9ntUbOk+k=