Skip to content

Commit e8978eb

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 e8978eb

File tree

2 files changed

+72
-6
lines changed

2 files changed

+72
-6
lines changed

client/nginx.go

+16-6
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,18 @@ type apiError struct {
112112
}
113113

114114
type internalError struct {
115-
err string
116-
apiError
115+
err string
116+
apiError apiError
117+
}
118+
119+
// Status returns the HTTP status code of the error.
120+
func (internalError *internalError) Status() int {
121+
return internalError.apiError.Status
122+
}
123+
124+
// Status returns the NGINX error code on the response.
125+
func (internalError *internalError) Code() string {
126+
return internalError.apiError.Code
117127
}
118128

119129
// Error allows internalError to match the Error interface.
@@ -1782,7 +1792,7 @@ func (client *NginxClient) GetStreamServerZones(ctx context.Context) (*StreamSer
17821792
if err != nil {
17831793
var ie *internalError
17841794
if errors.As(err, &ie) {
1785-
if ie.Code == pathNotFoundCode {
1795+
if ie.Code() == pathNotFoundCode {
17861796
return &zones, nil
17871797
}
17881798
}
@@ -1808,7 +1818,7 @@ func (client *NginxClient) GetStreamUpstreams(ctx context.Context) (*StreamUpstr
18081818
if err != nil {
18091819
var ie *internalError
18101820
if errors.As(err, &ie) {
1811-
if ie.Code == pathNotFoundCode {
1821+
if ie.Code() == pathNotFoundCode {
18121822
return &upstreams, nil
18131823
}
18141824
}
@@ -1824,7 +1834,7 @@ func (client *NginxClient) GetStreamZoneSync(ctx context.Context) (*StreamZoneSy
18241834
if err != nil {
18251835
var ie *internalError
18261836
if errors.As(err, &ie) {
1827-
if ie.Code == pathNotFoundCode {
1837+
if ie.Code() == pathNotFoundCode {
18281838
return nil, nil
18291839
}
18301840
}
@@ -2137,7 +2147,7 @@ func (client *NginxClient) GetStreamConnectionsLimit(ctx context.Context) (*Stre
21372147
if err != nil {
21382148
var ie *internalError
21392149
if errors.As(err, &ie) {
2140-
if ie.Code == pathNotFoundCode {
2150+
if ie.Code() == pathNotFoundCode {
21412151
return &limitConns, nil
21422152
}
21432153
}

client/nginx_test.go

+56
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,61 @@ func TestUpdateStreamServers(t *testing.T) {
14381439
}
14391440
}
14401441

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

0 commit comments

Comments
 (0)