Skip to content

Commit d169934

Browse files
committed
Make status code and NGINX error codes on internalError publicly accessible
This will allow users of the plus client to check errors and decide what further action to take. This fix follows the advice of commentators who counsel users of go to assert errors for behavior, not type. https://dave.cheney.net/2014/12/24/inspecting-errors The user would create a simple error interface including Status() and Code() methods and then could use errors.As() to cast the internalError to their own interface type. For example, if the user attempts to delete an upstream server using the client, the user can check the error returned for a http.StatusNotFound code, and if present, can make their application take no further action. If some error status code is returned, then the user can retry the delete. Previously in order to perform this kind of analysis the user would have to resort to string checking on the error: never a good solution.
1 parent 177be93 commit d169934

File tree

2 files changed

+82
-6
lines changed

2 files changed

+82
-6
lines changed

client/nginx.go

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ var (
5252
ErrPlusVersionNotFound = errors.New("plus version not found in the input string")
5353
)
5454

55+
// StatusError is an interface that defines our API with consumers of the plus client errors.
56+
// The error will return a http status code and an NGINX error code.
57+
type StatusError interface {
58+
Status() int
59+
Code() string
60+
}
61+
62+
var _ StatusError = (*internalError)(nil)
63+
5564
// NginxClient lets you access NGINX Plus API.
5665
type NginxClient struct {
5766
httpClient *http.Client
@@ -112,8 +121,18 @@ type apiError struct {
112121
}
113122

114123
type internalError struct {
115-
err string
116-
apiError
124+
err string
125+
apiError apiError
126+
}
127+
128+
// Status returns the HTTP status code of the error.
129+
func (internalError *internalError) Status() int {
130+
return internalError.apiError.Status
131+
}
132+
133+
// Status returns the NGINX error code on the response.
134+
func (internalError *internalError) Code() string {
135+
return internalError.apiError.Code
117136
}
118137

119138
// Error allows internalError to match the Error interface.
@@ -1782,7 +1801,7 @@ func (client *NginxClient) GetStreamServerZones(ctx context.Context) (*StreamSer
17821801
if err != nil {
17831802
var ie *internalError
17841803
if errors.As(err, &ie) {
1785-
if ie.Code == pathNotFoundCode {
1804+
if ie.Code() == pathNotFoundCode {
17861805
return &zones, nil
17871806
}
17881807
}
@@ -1808,7 +1827,7 @@ func (client *NginxClient) GetStreamUpstreams(ctx context.Context) (*StreamUpstr
18081827
if err != nil {
18091828
var ie *internalError
18101829
if errors.As(err, &ie) {
1811-
if ie.Code == pathNotFoundCode {
1830+
if ie.Code() == pathNotFoundCode {
18121831
return &upstreams, nil
18131832
}
18141833
}
@@ -1824,7 +1843,7 @@ func (client *NginxClient) GetStreamZoneSync(ctx context.Context) (*StreamZoneSy
18241843
if err != nil {
18251844
var ie *internalError
18261845
if errors.As(err, &ie) {
1827-
if ie.Code == pathNotFoundCode {
1846+
if ie.Code() == pathNotFoundCode {
18281847
return nil, nil
18291848
}
18301849
}
@@ -2137,7 +2156,7 @@ func (client *NginxClient) GetStreamConnectionsLimit(ctx context.Context) (*Stre
21372156
if err != nil {
21382157
var ie *internalError
21392158
if errors.As(err, &ie) {
2140-
if ie.Code == pathNotFoundCode {
2159+
if ie.Code() == pathNotFoundCode {
21412160
return &limitConns, nil
21422161
}
21432162
}

client/nginx_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package client
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"net/http"
78
"net/http/httptest"
89
"reflect"
@@ -1438,6 +1439,62 @@ func TestUpdateStreamServers(t *testing.T) {
14381439
}
14391440
}
14401441

1442+
func TestInternalError(t *testing.T) {
1443+
t.Parallel()
1444+
1445+
// mimic a user-defined interface type
1446+
type TestStatusError interface {
1447+
Status() int
1448+
Code() string
1449+
}
1450+
1451+
//nolint
1452+
anotherErr := errors.New("another error")
1453+
1454+
notFoundErr := &internalError{
1455+
err: "not found error",
1456+
apiError: apiError{
1457+
Text: "not found error",
1458+
Status: http.StatusNotFound,
1459+
Code: "not found code",
1460+
},
1461+
}
1462+
1463+
testcases := map[string]struct {
1464+
inputErr error
1465+
expectedStatus int
1466+
}{
1467+
"simple not found": {
1468+
inputErr: notFoundErr,
1469+
expectedStatus: http.StatusNotFound,
1470+
},
1471+
"not found joined with another error": {
1472+
inputErr: errors.Join(notFoundErr, anotherErr),
1473+
expectedStatus: http.StatusNotFound,
1474+
},
1475+
"not found wrapped with another error": {
1476+
inputErr: notFoundErr.Wrap("some error"),
1477+
expectedStatus: http.StatusNotFound,
1478+
},
1479+
}
1480+
1481+
for name, tc := range testcases {
1482+
t.Run(name, func(t *testing.T) {
1483+
t.Parallel()
1484+
1485+
var se TestStatusError
1486+
ok := errors.As(tc.inputErr, &se)
1487+
if !ok {
1488+
t.Fatalf("could not cast error %v as StatusError", tc.inputErr)
1489+
}
1490+
1491+
if se.Status() != tc.expectedStatus {
1492+
t.Fatalf("expected status %d, got status %d", tc.expectedStatus, se.Status())
1493+
}
1494+
})
1495+
}
1496+
}
1497+
14411498
type response struct {
14421499
servers interface{}
14431500
statusCode int

0 commit comments

Comments
 (0)