From bc16a6f55b3120301ee8252f36473e924da05938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kuba=20Tu=C5=BCnik?= Date: Wed, 27 Nov 2024 12:34:32 +0100 Subject: [PATCH] CA: wrap the provided errors in ToAutoscalerError() and AddPrefix(), implement Unwrap() This allows using errors.Is() to check if an AutoscalerError wraps a sentinel error (e.g. cloudprovider.ErrNotImplemented) when a prefix is added to it. --- .../scaleup/orchestrator/orchestrator_test.go | 7 ++-- cluster-autoscaler/utils/errors/errors.go | 34 +++++++++++++------ 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go b/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go index a1cdd15eba91..7790d7cba534 100644 --- a/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go +++ b/cluster-autoscaler/core/scaleup/orchestrator/orchestrator_test.go @@ -1835,11 +1835,8 @@ func TestAuthErrorHandling(t *testing.T) { Options: &defaultOptions, } results := runSimpleScaleUpTest(t, config) - expected := errors.NewAutoscalerError( - errors.AutoscalerErrorType("authError"), - "failed to increase node group size: auth error", - ) - assert.Equal(t, expected, results.ScaleUpError) + assert.Equal(t, errors.AutoscalerErrorType("authError"), results.ScaleUpError.Type()) + assert.Equal(t, "failed to increase node group size: auth error", results.ScaleUpError.Error()) assertLegacyRegistryEntry(t, "cluster_autoscaler_failed_scale_ups_total{reason=\"authError\"} 1") } diff --git a/cluster-autoscaler/utils/errors/errors.go b/cluster-autoscaler/utils/errors/errors.go index 58066e8ed939..bddf3d422120 100644 --- a/cluster-autoscaler/utils/errors/errors.go +++ b/cluster-autoscaler/utils/errors/errors.go @@ -41,8 +41,9 @@ type AutoscalerError interface { } type autoscalerErrorImpl struct { - errorType AutoscalerErrorType - msg string + errorType AutoscalerErrorType + wrappedErr error + msg string } const ( @@ -75,8 +76,9 @@ func NewAutoscalerError(errorType AutoscalerErrorType, msg string, args ...inter } } -// ToAutoscalerError converts an error to AutoscalerError with given type, +// ToAutoscalerError wraps an error to AutoscalerError with given type, // unless it already is an AutoscalerError (in which case it's not modified). +// errors.Is() works correctly on the wrapped error. func ToAutoscalerError(defaultType AutoscalerErrorType, err error) AutoscalerError { if e, ok := err.(AutoscalerError); ok { return e @@ -84,12 +86,25 @@ func ToAutoscalerError(defaultType AutoscalerErrorType, err error) AutoscalerErr if err == nil { return nil } - return NewAutoscalerError(defaultType, "%v", err) + return autoscalerErrorImpl{ + errorType: defaultType, + wrappedErr: err, + } } // Error implements golang error interface func (e autoscalerErrorImpl) Error() string { - return e.msg + msg := e.msg + if e.wrappedErr != nil { + msg = msg + e.wrappedErr.Error() + } + return msg +} + +// Unwrap returns the error wrapped via ToAutoscalerError or AddPrefix, so that errors.Is() works +// correctly. +func (e autoscalerErrorImpl) Unwrap() error { + return e.wrappedErr } // Type returns the type of AutoscalerError @@ -97,15 +112,14 @@ func (e autoscalerErrorImpl) Type() AutoscalerErrorType { return e.errorType } -// AddPrefix adds a prefix to error message. -// Returns the error it's called for convenient inline use. -// Example: +// AddPrefix returns an error wrapping this one, with the added prefix. errors.Is() works +// correctly on the wrapped error. +// Example usage: // if err := DoSomething(myObject); err != nil { // // return err.AddPrefix("can't do something with %v: ", myObject) // // } func (e autoscalerErrorImpl) AddPrefix(msg string, args ...interface{}) AutoscalerError { - e.msg = fmt.Sprintf(msg, args...) + e.msg - return e + return autoscalerErrorImpl{errorType: e.errorType, wrappedErr: e, msg: fmt.Sprintf(msg, args...)} }