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

fix(errors): support error checking methods for new problem structures #212

Merged
merged 1 commit into from
Mar 20, 2024
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
25 changes: 12 additions & 13 deletions core/base_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,10 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
httpResponse, err = service.Client.Do(req)

if err != nil {
errMsg := err.Error()
if strings.Contains(err.Error(), SSL_CERTIFICATION_ERROR) {
errMsg = ERRORMSG_SSL_VERIFICATION_FAILED + "\n" + errMsg
err = fmt.Errorf(ERRORMSG_SSL_VERIFICATION_FAILED + "\n" + err.Error())
}
err = SDKErrorf(nil, errMsg, "no-connection-made", getComponentInfo())
err = SDKErrorf(err, "", "no-connection-made", getComponentInfo())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pyrooka Norbert - I want to publicly acknowledge that "you told me so" with respect to saving the previously returned errors in case we ever needed them! 😂

Turns out, we do need them - so this PR is adding everything back in just like you recommended in my last PR 😛

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some developers never learn... 😂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, thanks for mentioning me, appreciate it! But I hope you didn't do this to protect yourself from future complications that might come up, so you can point at me and say "Hey, that was Norbert's idea, not mine..." 😂

return
}

Expand Down Expand Up @@ -452,8 +451,8 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
defer httpResponse.Body.Close() // #nosec G307
responseBody, readErr := io.ReadAll(httpResponse.Body)
if readErr != nil {
errMsg := fmt.Sprintf(ERRORMSG_READ_RESPONSE_BODY, readErr.Error())
err = SDKErrorf(nil, errMsg, "cant-read-success-res-body", getComponentInfo())
err = fmt.Errorf(ERRORMSG_READ_RESPONSE_BODY, readErr.Error())
err = SDKErrorf(err, "", "cant-read-success-res-body", getComponentInfo())
return
}

Expand All @@ -469,8 +468,8 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
if decodeErr != nil {
// Error decoding the response body.
// Return the response body in RawResult, along with an error.
errMsg := fmt.Sprintf(ERRORMSG_UNMARSHAL_RESPONSE_BODY, decodeErr.Error())
err = SDKErrorf(nil, errMsg, "res-body-decode-error", getComponentInfo())
err = fmt.Errorf(ERRORMSG_UNMARSHAL_RESPONSE_BODY, decodeErr.Error())
err = SDKErrorf(err, "", "res-body-decode-error", getComponentInfo())
detailedResponse.RawResult = responseBody
return
}
Expand Down Expand Up @@ -500,8 +499,8 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
// At this point, we don't know how to set the result field, so we have to return an error.
// But make sure we save the bytes we read in the DetailedResponse for debugging purposes
detailedResponse.Result = responseBody
errMsg := fmt.Sprintf(ERRORMSG_UNEXPECTED_RESPONSE, contentType, resultType)
err = SDKErrorf(nil, errMsg, "unparsable-result-field", getComponentInfo())
err = fmt.Errorf(ERRORMSG_UNEXPECTED_RESPONSE, contentType, resultType)
err = SDKErrorf(err, "", "unparsable-result-field", getComponentInfo())
return
}
}
Expand Down Expand Up @@ -602,7 +601,7 @@ type Error struct {
func decodeAsMap(byteBuffer []byte) (result map[string]interface{}, err error) {
err = json.NewDecoder(bytes.NewReader(byteBuffer)).Decode(&result)
if err != nil {
err = SDKErrorf(nil, err.Error(), "decode-error", getComponentInfo())
err = SDKErrorf(err, "", "decode-error", getComponentInfo())
}
return
}
Expand Down Expand Up @@ -836,17 +835,17 @@ func IBMCloudSDKRetryPolicy(ctx context.Context, resp *http.Response, err error)
if v, ok := err.(*url.Error); ok {
// Don't retry if the error was due to too many redirects.
if redirectsErrorRe.MatchString(v.Error()) {
return false, SDKErrorf(nil, v.Error(), "too-many-redirects", getComponentInfo())
return false, SDKErrorf(v, "", "too-many-redirects", getComponentInfo())
}

// Don't retry if the error was due to an invalid protocol scheme.
if schemeErrorRe.MatchString(v.Error()) {
return false, SDKErrorf(nil, v.Error(), "invalid-scheme", getComponentInfo())
return false, SDKErrorf(v, "", "invalid-scheme", getComponentInfo())
}

// Don't retry if the error was due to TLS cert verification failure.
if _, ok := v.Err.(x509.UnknownAuthorityError); ok {
return false, SDKErrorf(nil, v.Error(), "cert-failure", getComponentInfo())
return false, SDKErrorf(v, "", "cert-failure", getComponentInfo())
}
}

Expand Down
8 changes: 4 additions & 4 deletions core/container_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,8 @@ func (authenticator *ContainerAuthenticator) retrieveCRToken() (crToken string,
}

if err != nil {
errMsg := fmt.Sprintf(ERRORMSG_UNABLE_RETRIEVE_CRTOKEN, err.Error())
sdkErr := SDKErrorf(nil, errMsg, "no-cr-token", getComponentInfo())
err = fmt.Errorf(ERRORMSG_UNABLE_RETRIEVE_CRTOKEN, err.Error())
sdkErr := SDKErrorf(err, "", "no-cr-token", getComponentInfo())
GetLogger().Debug(sdkErr.GetDebugMessage())
err = sdkErr
return
Expand All @@ -514,8 +514,8 @@ func (authenticator *ContainerAuthenticator) readFile(filename string) (crToken
var bytes []byte
bytes, err = os.ReadFile(filename) // #nosec G304
if err != nil {
err = SDKErrorf(nil, err.Error(), "read-file-error", getComponentInfo())
GetLogger().Debug(err.Error())
err = SDKErrorf(err, "", "read-file-error", getComponentInfo())
GetLogger().Debug(err.(*SDKProblem).GetDebugMessage())
return
}

Expand Down
6 changes: 3 additions & 3 deletions core/cp4d_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func (authenticator *CloudPakForDataAuthenticator) requestToken() (tokenResponse
GetLogger().Debug("Invoking CP4D token service operation: %s", builder.URL)
resp, err := authenticator.client().Do(req)
if err != nil {
err = SDKErrorf(nil, err.Error(), "cp4d-request-error", getComponentInfo())
err = SDKErrorf(err, "", "cp4d-request-error", getComponentInfo())
return
}
GetLogger().Debug("Returned from CP4D token service operation, received status code %d", resp.StatusCode)
Expand Down Expand Up @@ -369,8 +369,8 @@ func (authenticator *CloudPakForDataAuthenticator) requestToken() (tokenResponse
err = json.NewDecoder(resp.Body).Decode(tokenResponse)
defer resp.Body.Close() // #nosec G307
if err != nil {
errMsg := fmt.Sprintf(ERRORMSG_UNMARSHAL_AUTH_RESPONSE, err.Error())
err = SDKErrorf(nil, errMsg, "cp4d-res-unmarshal-error", getComponentInfo())
err = fmt.Errorf(ERRORMSG_UNMARSHAL_AUTH_RESPONSE, err.Error())
err = SDKErrorf(err, "", "cp4d-res-unmarshal-error", getComponentInfo())
tokenResponse = nil
return
}
Expand Down
4 changes: 2 additions & 2 deletions core/datetime.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func ParseDate(dateString string) (fmtDate strfmt.Date, err error) {
if err == nil {
fmtDate = strfmt.Date(formattedTime)
} else {
err = SDKErrorf(nil, err.Error(), "date-parse-error", getComponentInfo())
err = SDKErrorf(err, "", "date-parse-error", getComponentInfo())
}
return
}
Expand All @@ -93,7 +93,7 @@ func ParseDate(dateString string) (fmtDate strfmt.Date, err error) {
func ParseDateTime(dateString string) (strfmt.DateTime, error) {
dt, err := strfmt.ParseDateTime(dateString)
if err != nil {
err = SDKErrorf(nil, err.Error(), "datetime-parse-error", getComponentInfo())
err = SDKErrorf(err, "", "datetime-parse-error", getComponentInfo())
}
return dt, err
}
2 changes: 1 addition & 1 deletion core/file_with_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func UnmarshalFileWithMetadata(m map[string]json.RawMessage, result interface{})
}
data, err = os.Open(pathToData) // #nosec G304
if err != nil {
err = SDKErrorf(nil, err.Error(), "file-open-error", getComponentInfo())
err = SDKErrorf(err, "", "file-open-error", getComponentInfo())
return
}
obj.Data = data
Expand Down
4 changes: 2 additions & 2 deletions core/gzip.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewGzipCompressionReader(uncompressedReader io.Reader) (io.Reader, error) {
// to the pipe only when the pipe reader is called to retrieve more bytes.
_, err := io.Copy(compressedWriter, uncompressedReader)
if err != nil {
sdkErr := SDKErrorf(nil, err.Error(), "compression-failed", getComponentInfo())
sdkErr := SDKErrorf(err, "", "compression-failed", getComponentInfo())
_ = pipeWriter.CloseWithError(sdkErr)
}
}()
Expand All @@ -54,7 +54,7 @@ func NewGzipCompressionReader(uncompressedReader io.Reader) (io.Reader, error) {
func NewGzipDecompressionReader(compressedReader io.Reader) (io.Reader, error) {
res, err := gzip.NewReader(compressedReader)
if err != nil {
err = SDKErrorf(nil, err.Error(), "decompress-read-error", getComponentInfo())
err = SDKErrorf(err, "", "decompress-read-error", getComponentInfo())
}
return res, err
}
7 changes: 7 additions & 0 deletions core/http_problem.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ func (e *HTTPProblem) GetID() string {
return CreateIDHash("http", e.GetBaseSignature(), e.OperationID, fmt.Sprint(e.Response.GetStatusCode()))
}

// Is allows an HTTPProblem instance to be compared against another error for equality.
// An HTTPProblem is considered equal to another error if 1) the error is also a Problem and
// 2) it has the same ID (i.e. it is the same problem scenario).
func (e *HTTPProblem) Is(target error) bool {
return is(target, e.GetID())
}

func (e *HTTPProblem) getErrorCode() string {
// If the error response was a standard JSON body, the result will
// be a map and we can do a decent job of guessing the code.
Expand Down
12 changes: 12 additions & 0 deletions core/http_problem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package core
// limitations under the License.

import (
"errors"
"net/http"
"testing"

Expand Down Expand Up @@ -220,6 +221,17 @@ func TestHTTPProblemGetErrorCode(t *testing.T) {
assert.Equal(t, "invalid-input", httpProb.getErrorCode())
}

func TestHTTPProblemIsWithProblem(t *testing.T) {
firstProb := httpErrorf("Bad request", getPopulatedDetailedResponse())
EnrichHTTPProblem(firstProb, "create_resource", NewProblemComponent("service", "1.0.0"))

secondProb := httpErrorf("Invalid input", getPopulatedDetailedResponse())
EnrichHTTPProblem(secondProb, "create_resource", NewProblemComponent("service", "1.2.3"))

assert.NotEqual(t, firstProb, secondProb)
assert.True(t, errors.Is(firstProb, secondProb))
}

func TestHTTPErrorf(t *testing.T) {
message := "Bad request"
httpProb := httpErrorf(message, getPopulatedDetailedResponse())
Expand Down
2 changes: 1 addition & 1 deletion core/iam_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ func (authenticator *IamAuthenticator) RequestToken() (*IamTokenServerResponse,
GetLogger().Debug("Invoking IAM 'get token' operation: %s", builder.URL)
resp, err := authenticator.client().Do(req)
if err != nil {
err = SDKErrorf(nil, err.Error(), "request-error", getComponentInfo())
err = SDKErrorf(err, "", "request-error", getComponentInfo())
return nil, err
}
GetLogger().Debug("Returned from IAM 'get token' operation, received status code %d", resp.StatusCode)
Expand Down
22 changes: 20 additions & 2 deletions core/ibm_problem.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ type IBMProblem struct {
// causedBy allows for the storage of a problem from a previous component,
// if there is one.
causedBy Problem

// nativeCausedBy allows for the storage of an error that is the cause of
// the problem instance but is not a part of the official chain of problem
// types. By including these errors in the "Unwrap" chain, the problem type
// changes become compatible with downstream code that uses error checking
// methods like "Is" and "As".
nativeCausedBy error
}

// Error returns the problem's message and implements the native
Expand Down Expand Up @@ -109,12 +116,21 @@ func (e *IBMProblem) GetCausedBy() Problem {
// it does not include the error instance the method is called on - that is
// looked at separately by the "errors" package in functions like "As".
func (e *IBMProblem) Unwrap() []error {
var errs []error
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to this method are the key component of this PR. These changes re-enable compatibility with methods like errors.Is by including the previously returned errors in the list of "unwrapped" errors.

The changes to add Is implementations to the problem types are purely extra - they allow for comparisons against problems that go beyond being the same instance. They enable equality for the same problem scenario (i.e. same ID) which is what the new types are all about.


// Include native (i.e. non-Problem) caused by errors in the
// chain for compatibility with respect to downstream methods
// like "errors.Is" or "errors.As".
if e.nativeCausedBy != nil {
errs = append(errs, e.nativeCausedBy)
}

causedBy := e.GetCausedBy()
if causedBy == nil {
return nil
return errs
}

errs := []error{causedBy}
errs = append(errs, causedBy)

var toUnwrap interface{ Unwrap() []error }
if errors.As(causedBy, &toUnwrap) {
Expand Down Expand Up @@ -144,6 +160,8 @@ func ibmProblemf(err error, severity problemSeverity, component *ProblemComponen
var causedBy Problem
if errors.As(err, &causedBy) {
newError.causedBy = causedBy
} else {
newError.nativeCausedBy = err
}

return newError
Expand Down
5 changes: 3 additions & 2 deletions core/jwt_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ func parseJWT(tokenString string) (claims *coreJWTClaims, err error) {
claims = &coreJWTClaims{}
err = json.Unmarshal(claimBytes, claims)
if err != nil {
err = SDKErrorf(nil, fmt.Sprintf("error unmarshalling token: %s", err.Error()), "bad-token", getComponentInfo())
err = fmt.Errorf("error unmarshalling token: %s", err.Error())
err = SDKErrorf(err, "", "bad-token", getComponentInfo())
return
}

Expand All @@ -63,7 +64,7 @@ func decodeSegment(seg string) ([]byte, error) {

res, err := base64.URLEncoding.DecodeString(seg)
if err != nil {
err = SDKErrorf(nil, fmt.Sprintf("error decoding claims segment: %s", err.Error()), "bad-claim-seg", getComponentInfo())
err = SDKErrorf(err, fmt.Sprintf("error decoding claims segment: %s", err.Error()), "bad-claim-seg", getComponentInfo())
}
return res, err
}
2 changes: 1 addition & 1 deletion core/mcsp_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func (authenticator *MCSPAuthenticator) RequestToken() (*MCSPTokenServerResponse
GetLogger().Debug("Invoking MCSP 'get token' operation: %s", builder.URL)
resp, err := authenticator.client().Do(req)
if err != nil {
err = SDKErrorf(nil, err.Error(), "request-error", getComponentInfo())
err = SDKErrorf(err, "", "request-error", getComponentInfo())
return nil, err
}
GetLogger().Debug("Returned from MCSP 'get token' operation, received status code %d", resp.StatusCode)
Expand Down
10 changes: 10 additions & 0 deletions core/problem_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package core
import (
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -55,3 +56,12 @@ func getProblemInfoAsYAML(orderedMaps *OrderedMaps) string {
func getComponentInfo() *ProblemComponent {
return NewProblemComponent(MODULE_NAME, __VERSION__)
}

// is provides a simple utility function that assists problem types
// implement an "Is" function for checking error equality. Error types
// are treated as equivalent if they are both Problem types and their
// IDs match.
func is(target error, id string) bool {
var problem Problem
return errors.As(target, &problem) && problem.GetID() == id
}
22 changes: 13 additions & 9 deletions core/request_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ func (requestBuilder *RequestBuilder) ConstructHTTPURL(serviceURL string, pathSe

URL, err := url.Parse(serviceURL)
if err != nil {
errMsg := fmt.Sprintf(ERRORMSG_SERVICE_URL_INVALID, err.Error())
return requestBuilder, SDKErrorf(nil, errMsg, "bad-url", getComponentInfo())
err := fmt.Errorf(ERRORMSG_SERVICE_URL_INVALID, err.Error())
return requestBuilder, SDKErrorf(err, "", "bad-url", getComponentInfo())
}

for i, pathSegment := range pathSegments {
Expand All @@ -122,8 +122,8 @@ func (requestBuilder *RequestBuilder) ConstructHTTPURL(serviceURL string, pathSe

if pathParameters != nil && i < len(pathParameters) {
if pathParameters[i] == "" {
errMsg := fmt.Sprintf(ERRORMSG_PATH_PARAM_EMPTY, fmt.Sprintf("[%d]", i))
return requestBuilder, SDKErrorf(nil, errMsg, "empty-path-param", getComponentInfo())
err := fmt.Errorf(ERRORMSG_PATH_PARAM_EMPTY, fmt.Sprintf("[%d]", i))
return requestBuilder, SDKErrorf(err, "", "empty-path-param", getComponentInfo())
}
URL.Path += "/" + pathParameters[i]
}
Expand Down Expand Up @@ -186,8 +186,8 @@ func (requestBuilder *RequestBuilder) ResolveRequestURL(serviceURL string, path

URL, err := url.Parse(urlString)
if err != nil {
errMsg := fmt.Sprintf(ERRORMSG_SERVICE_URL_INVALID, err.Error())
return requestBuilder, SDKErrorf(nil, errMsg, "bad-url", getComponentInfo())
err = fmt.Errorf(ERRORMSG_SERVICE_URL_INVALID, err.Error())
return requestBuilder, SDKErrorf(err, "", "bad-url", getComponentInfo())
}

requestBuilder.URL = URL
Expand Down Expand Up @@ -231,8 +231,8 @@ func (requestBuilder *RequestBuilder) SetBodyContentJSON(bodyContent interface{}
requestBuilder.Body = new(bytes.Buffer)
err := json.NewEncoder(requestBuilder.Body.(io.Writer)).Encode(bodyContent)
if err != nil {
errMsg := fmt.Sprintf("Could not encode JSON body:\n%s", err.Error())
err = SDKErrorf(nil, errMsg, "bad-encode", getComponentInfo())
err = fmt.Errorf("Could not encode JSON body:\n%s", err.Error())
err = SDKErrorf(err, "", "bad-encode", getComponentInfo())
}
return requestBuilder, err
}
Expand Down Expand Up @@ -265,7 +265,7 @@ func createFormFile(formWriter *multipart.Writer, fieldname string, filename str

res, err := formWriter.CreatePart(h)
if err != nil {
err = SDKErrorf(nil, err.Error(), "create-part-error", getComponentInfo())
err = SDKErrorf(err, "", "create-part-error", getComponentInfo())
}
return res, err
}
Expand Down Expand Up @@ -494,12 +494,16 @@ func (requestBuilder *RequestBuilder) SetBodyContent(contentType string, jsonCon
// which should be a "string", "*string" or an "io.Reader"
if str, ok := nonJSONContent.(string); ok {
builder, err = requestBuilder.SetBodyContentString(str)
err = RepurposeSDKProblem(err, "set-body-string-error")
} else if strPtr, ok := nonJSONContent.(*string); ok {
builder, err = requestBuilder.SetBodyContentString(*strPtr)
err = RepurposeSDKProblem(err, "set-body-strptr-error")
} else if stream, ok := nonJSONContent.(io.Reader); ok {
builder, err = requestBuilder.SetBodyContentStream(stream)
err = RepurposeSDKProblem(err, "set-body-reader-error")
} else if stream, ok := nonJSONContent.(*io.ReadCloser); ok {
builder, err = requestBuilder.SetBodyContentStream(*stream)
err = RepurposeSDKProblem(err, "set-body-readerptr-error")
} else {
builder = requestBuilder
errMsg := fmt.Sprintf("Invalid type for non-JSON body content: %s", reflect.TypeOf(nonJSONContent).String())
Expand Down
7 changes: 7 additions & 0 deletions core/sdk_problem.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ func (e *SDKProblem) GetID() string {
return CreateIDHash("sdk", e.GetBaseSignature(), e.Function)
}

// Is allows an SDKProblem instance to be compared against another error for equality.
// An SDKProblem is considered equal to another error if 1) the error is also a Problem and
// 2) it has the same ID (i.e. it is the same problem scenario).
func (e *SDKProblem) Is(target error) bool {
return is(target, e.GetID())
}

// GetConsoleOrderedMaps returns an ordered-map representation
// of an SDKProblem instance suited for a console message.
func (e *SDKProblem) GetConsoleOrderedMaps() *OrderedMaps {
Expand Down
Loading