From c3a0ef23eb33b1f36d52a666d9ff9778c3040fa5 Mon Sep 17 00:00:00 2001
From: Alex Russell <a.russell@f5.com>
Date: Tue, 25 Mar 2025 16:24:19 -0600
Subject: [PATCH] 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.
---
 client/nginx.go      | 31 +++++++++++++++++----
 client/nginx_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 6 deletions(-)

diff --git a/client/nginx.go b/client/nginx.go
index 4a44d98c..ebe59212 100644
--- a/client/nginx.go
+++ b/client/nginx.go
@@ -52,6 +52,15 @@ var (
 	ErrPlusVersionNotFound = errors.New("plus version not found in the input string")
 )
 
+// StatusError is an interface that defines our API with consumers of the plus client errors.
+// The error will return a http status code and an NGINX error code.
+type StatusError interface {
+	Status() int
+	Code() string
+}
+
+var _ StatusError = (*internalError)(nil)
+
 // NginxClient lets you access NGINX Plus API.
 type NginxClient struct {
 	httpClient  *http.Client
@@ -112,8 +121,18 @@ type apiError struct {
 }
 
 type internalError struct {
-	err string
-	apiError
+	err      string
+	apiError apiError
+}
+
+// Status returns the HTTP status code of the error.
+func (internalError *internalError) Status() int {
+	return internalError.apiError.Status
+}
+
+// Status returns the NGINX error code on the response.
+func (internalError *internalError) Code() string {
+	return internalError.apiError.Code
 }
 
 // Error allows internalError to match the Error interface.
@@ -1782,7 +1801,7 @@ func (client *NginxClient) GetStreamServerZones(ctx context.Context) (*StreamSer
 	if err != nil {
 		var ie *internalError
 		if errors.As(err, &ie) {
-			if ie.Code == pathNotFoundCode {
+			if ie.Code() == pathNotFoundCode {
 				return &zones, nil
 			}
 		}
@@ -1808,7 +1827,7 @@ func (client *NginxClient) GetStreamUpstreams(ctx context.Context) (*StreamUpstr
 	if err != nil {
 		var ie *internalError
 		if errors.As(err, &ie) {
-			if ie.Code == pathNotFoundCode {
+			if ie.Code() == pathNotFoundCode {
 				return &upstreams, nil
 			}
 		}
@@ -1824,7 +1843,7 @@ func (client *NginxClient) GetStreamZoneSync(ctx context.Context) (*StreamZoneSy
 	if err != nil {
 		var ie *internalError
 		if errors.As(err, &ie) {
-			if ie.Code == pathNotFoundCode {
+			if ie.Code() == pathNotFoundCode {
 				return nil, nil
 			}
 		}
@@ -2137,7 +2156,7 @@ func (client *NginxClient) GetStreamConnectionsLimit(ctx context.Context) (*Stre
 	if err != nil {
 		var ie *internalError
 		if errors.As(err, &ie) {
-			if ie.Code == pathNotFoundCode {
+			if ie.Code() == pathNotFoundCode {
 				return &limitConns, nil
 			}
 		}
diff --git a/client/nginx_test.go b/client/nginx_test.go
index 46467477..8d565cae 100644
--- a/client/nginx_test.go
+++ b/client/nginx_test.go
@@ -3,6 +3,7 @@ package client
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"reflect"
@@ -1438,6 +1439,70 @@ func TestUpdateStreamServers(t *testing.T) {
 	}
 }
 
+func TestInternalError(t *testing.T) {
+	t.Parallel()
+
+	// mimic a user-defined interface type
+	type TestStatusError interface {
+		Status() int
+		Code() string
+	}
+
+	//nolint // ignore golangci-lint err113 sugggestion to create package level static error
+	anotherErr := errors.New("another error")
+
+	notFoundErr := &internalError{
+		err: "not found error",
+		apiError: apiError{
+			Text:   "not found error",
+			Status: http.StatusNotFound,
+			Code:   "not found code",
+		},
+	}
+
+	testcases := map[string]struct {
+		inputErr       error
+		expectedCode   string
+		expectedStatus int
+	}{
+		"simple not found": {
+			inputErr:       notFoundErr,
+			expectedStatus: http.StatusNotFound,
+			expectedCode:   "not found code",
+		},
+		"not found joined with another error": {
+			inputErr:       errors.Join(notFoundErr, anotherErr),
+			expectedStatus: http.StatusNotFound,
+			expectedCode:   "not found code",
+		},
+		"not found wrapped with another error": {
+			inputErr:       notFoundErr.Wrap("some error"),
+			expectedStatus: http.StatusNotFound,
+			expectedCode:   "not found code",
+		},
+	}
+
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			var se TestStatusError
+			ok := errors.As(tc.inputErr, &se)
+			if !ok {
+				t.Fatalf("could not cast error %v as StatusError", tc.inputErr)
+			}
+
+			if se.Status() != tc.expectedStatus {
+				t.Fatalf("expected status %d, got status %d", tc.expectedStatus, se.Status())
+			}
+
+			if se.Code() != tc.expectedCode {
+				t.Fatalf("expected code %s, got code %s", tc.expectedCode, se.Code())
+			}
+		})
+	}
+}
+
 type response struct {
 	servers    interface{}
 	statusCode int