From 21ea50a4e0ff1f8fbb1cdf4dd69d10d0d43f2658 Mon Sep 17 00:00:00 2001 From: Waldemar Quevedo Date: Sun, 14 Aug 2022 11:19:13 -0700 Subject: [PATCH] js: smaller JetStreamError interface, make JetStreamAPIError concrete type Signed-off-by: Waldemar Quevedo --- jsm.go | 86 +++++++++++++++++-------------------------------- nats.go | 14 ++++++-- test/js_test.go | 40 +++++++++++++---------- 3 files changed, 65 insertions(+), 75 deletions(-) diff --git a/jsm.go b/jsm.go index 82cb5d348..9bb7a7909 100644 --- a/jsm.go +++ b/jsm.go @@ -154,22 +154,22 @@ type ExternalStream struct { DeliverPrefix string `json:"deliver"` } -// APIError is included in all API responses if there was an error. -type APIError struct { +// JetStreamAPIError is included in all API responses if there was an error. +type JetStreamAPIError struct { Code int `json:"code"` ErrorCode ErrorCode `json:"err_code"` Description string `json:"description,omitempty"` } // Error prints the JetStream API error code and description -func (e *APIError) Error() string { +func (e *JetStreamAPIError) Error() string { return fmt.Sprintf("nats: API error %d: %s", e.ErrorCode, e.Description) } -// Is matches against an APIError. -func (e *APIError) Is(err error) bool { +// Is matches against an JetStreamAPIError. +func (e *JetStreamAPIError) Is(err error) bool { // Extract internal APIError to match against. - var aerr *APIError + var aerr *JetStreamAPIError ok := errors.As(err, &aerr) if !ok { return ok @@ -177,53 +177,37 @@ func (e *APIError) Is(err error) bool { return e.ErrorCode == aerr.ErrorCode } -// JetStreamAPIError is an error result from making a request to the -// JetStream API. -type JetStreamAPIError interface { - Code() int - ErrorCode() ErrorCode - Description() string +// JetStreamError is an error result that happens when using JetStream. +type JetStreamError interface { + APIError() *JetStreamAPIError error } -type jsAPIError struct { - code int - errorCode ErrorCode - description string - message string +type jsError struct { + apiErr *JetStreamAPIError + message string } -func (err *jsAPIError) Code() int { - return err.code +func (err *jsError) APIError() *JetStreamAPIError { + return err.apiErr } -func (err *jsAPIError) ErrorCode() ErrorCode { - return err.errorCode -} - -func (err *jsAPIError) Description() string { - if err.description == "" { - return err.message +func (err *jsError) Error() string { + if err.apiErr != nil && err.apiErr.Description != "" { + return fmt.Sprintf("nats: %v", err.apiErr.Description) } - return err.description -} - -func (err *jsAPIError) Error() string { return fmt.Sprintf("nats: %v", err.message) } -func (err *jsAPIError) Unwrap() error { - return &APIError{ - Code: err.Code(), - ErrorCode: err.ErrorCode(), - Description: err.Description(), - } +func (err *jsError) Unwrap() error { + // Allow matching to embedded APIError in case there is one. + return err.apiErr } // apiResponse is a standard response from the JetStream JSON API type apiResponse struct { - Type string `json:"type"` - Error *APIError `json:"error,omitempty"` + Type string `json:"type"` + Error *JetStreamAPIError `json:"error,omitempty"` } // apiPaged includes variables used to create paged responses from the JSON API @@ -294,14 +278,6 @@ const ( JSErrCodeMessageNotFound ErrorCode = 10037 ) -var ( - // ErrJetStreamNotEnabled is an error returned when JetStream is not enabled. - ErrJetStreamNotEnabledForAccount JetStreamAPIError = &jsAPIError{errorCode: JSErrCodeJetStreamNotEnabledForAccount, description: "nats: jetstream not enabled for account"} - - // ErrJetStreamNotEnabledForAccount is an error returned when JetStream is not enabled for an account. - ErrJetStreamNotEnabled JetStreamAPIError = &jsAPIError{errorCode: JSErrCodeJetStreamNotEnabled, description: "nats: jetstream not enabled"} -) - // AccountInfo retrieves info about the JetStream usage from the current account. // If JetStream is not enabled, this will return ErrJetStreamNotEnabled // Other errors can happen but are generally considered retryable @@ -330,12 +306,10 @@ func (js *js) AccountInfo(opts ...JSOpt) (*AccountInfo, error) { var err error // Internally checks based on error code instead of description match. if errors.Is(info.Error, ErrJetStreamNotEnabledForAccount) { - err = ErrJetStreamNotEnabled + err = ErrJetStreamNotEnabledForAccount } else { - err = &jsAPIError{ - code: info.Error.Code, - errorCode: info.Error.ErrorCode, - description: info.Error.Description, + err = &jsError{ + apiErr: info.Error, } } return nil, err @@ -829,11 +803,11 @@ type StreamInfo struct { // StreamSourceInfo shows information about an upstream stream source. type StreamSourceInfo struct { - Name string `json:"name"` - Lag uint64 `json:"lag"` - Active time.Duration `json:"active"` - External *ExternalStream `json:"external"` - Error *APIError `json:"error"` + Name string `json:"name"` + Lag uint64 `json:"lag"` + Active time.Duration `json:"active"` + External *ExternalStream `json:"external"` + Error *JetStreamAPIError `json:"error"` } // StreamState is information about the given stream. diff --git a/nats.go b/nats.go index 30dff008d..7c9264c1f 100644 --- a/nats.go +++ b/nats.go @@ -153,14 +153,13 @@ var ( ErrStreamNameRequired = errors.New("nats: stream name is required") ErrStreamNotFound = errors.New("nats: stream not found") ErrConsumerNotFound = errors.New("nats: consumer not found") + ErrConsumerNameAlreadyInUse = errors.New("nats: consumer name already in use") ErrConsumerNameRequired = errors.New("nats: consumer name is required") ErrConsumerConfigRequired = errors.New("nats: consumer configuration is required") ErrStreamSnapshotConfigRequired = errors.New("nats: stream snapshot configuration is required") ErrDeliverSubjectRequired = errors.New("nats: deliver subject is required") ErrPullSubscribeToPushConsumer = errors.New("nats: cannot pull subscribe to push based consumer") ErrPullSubscribeRequired = errors.New("nats: must use pull subscribe to bind to pull based consumer") - ErrConsumerNameAlreadyInUse = errors.New("nats: consumer name already in use") - ErrConsumerNotActive = errors.New("nats: consumer not active") ErrMsgNotFound = errors.New("nats: message not found") ErrMsgAlreadyAckd = errors.New("nats: message was already acknowledged") ErrCantAckIfConsumerAckNone = errors.New("nats: cannot acknowledge a message for a consumer with AckNone policy") @@ -171,6 +170,17 @@ var ( ErrConnectionNotTLS = errors.New("nats: connection is not tls") ) +var ( + // ErrJetStreamNotEnabled is an error returned when JetStream is not enabled for an account. + ErrJetStreamNotEnabled JetStreamError = &jsError{apiErr: &JetStreamAPIError{ErrorCode: JSErrCodeJetStreamNotEnabled, Description: "jetstream not enabled"}} + + // ErrJetStreamNotEnabledForAccount is an error returned when JetStream is not enabled for an account. + ErrJetStreamNotEnabledForAccount JetStreamError = &jsError{apiErr: &JetStreamAPIError{ErrorCode: JSErrCodeJetStreamNotEnabledForAccount, Description: "jetstream not enabled"}} + + // ErrConsumerNotActive is an error returned when consumer is not active. + ErrConsumerNotActive JetStreamError = &jsError{message: "consumer not active"} +) + func init() { rand.Seed(time.Now().UnixNano()) } diff --git a/test/js_test.go b/test/js_test.go index 10614ccf8..df42a155c 100644 --- a/test/js_test.go +++ b/test/js_test.go @@ -94,8 +94,8 @@ func TestJetStreamNotAccountEnabled(t *testing.T) { } // matching via errors.Is - if ok := errors.Is(err, nats.ErrJetStreamNotEnabled); !ok { - t.Fatal("Expected ErrJetStreamNotEnabled") + if ok := errors.Is(err, nats.ErrJetStreamNotEnabledForAccount); !ok { + t.Fatal("Expected ErrJetStreamNotEnabledForAccount") } // matching wrapped via error.Is @@ -105,23 +105,29 @@ func TestJetStreamNotAccountEnabled(t *testing.T) { } // via classic type assertion. - jserr, ok := err.(nats.JetStreamAPIError) + jserr, ok := err.(nats.JetStreamError) if !ok { - t.Fatal("Expected a JetStreamAPIError") + t.Fatal("Expected a JetStreamError") } expected := nats.JSErrCodeJetStreamNotEnabledForAccount - if jserr.ErrorCode() != nats.ErrorCode(expected) { - t.Fatalf("Expected: %v, got: %v", expected, jserr.ErrorCode()) + if jserr.APIError().ErrorCode != expected { + t.Fatalf("Expected: %v, got: %v", expected, jserr.APIError().ErrorCode) + } + if jserr.APIError() == nil { + t.Fatal("Expected APIError") } // matching to interface via errors.As(...) - var apierr nats.JetStreamAPIError + var apierr nats.JetStreamError ok = errors.As(err, &apierr) if !ok { - t.Fatal("Expected a JetStreamAPIError") + t.Fatal("Expected a JetStreamError") + } + if apierr.APIError() == nil { + t.Fatal("Expected APIError") } - if apierr.ErrorCode() != expected { - t.Fatalf("Expected: %v, got: %v", expected, apierr.ErrorCode()) + if apierr.APIError().ErrorCode != expected { + t.Fatalf("Expected: %v, got: %v", expected, apierr.APIError().ErrorCode) } expectedMessage := "nats: jetstream not enabled" if apierr.Error() != expectedMessage { @@ -129,17 +135,17 @@ func TestJetStreamNotAccountEnabled(t *testing.T) { } // matching arbitrary custom error via errors.Is(...) - customErr := &nats.APIError{ErrorCode: expected} - if ok := errors.Is(customErr, nats.ErrJetStreamNotEnabled); !ok { - t.Fatal("Expected wrapped ErrJetStreamNotEnabled") + customErr := &nats.JetStreamAPIError{ErrorCode: expected} + if ok := errors.Is(customErr, nats.ErrJetStreamNotEnabledForAccount); !ok { + t.Fatal("Expected wrapped ErrJetStreamNotEnabledForAccount") } - customErr = &nats.APIError{ErrorCode: 1} - if ok := errors.Is(customErr, nats.ErrJetStreamNotEnabled); ok { + customErr = &nats.JetStreamAPIError{ErrorCode: 1} + if ok := errors.Is(customErr, nats.ErrJetStreamNotEnabledForAccount); ok { t.Fatal("Expected to not match ErrJetStreamNotEnabled") } // matching to concrete type via errors.As(...) - var aerr *nats.APIError + var aerr *nats.JetStreamAPIError ok = errors.As(err, &aerr) if !ok { t.Fatal("Expected an APIError") @@ -4894,7 +4900,7 @@ func testJetStreamMirror_Source(t *testing.T, nodes ...*jsServer) { if err == nil { t.Fatal("Unexpected success") } - apiErr := &nats.APIError{} + apiErr := &nats.JetStreamAPIError{} if !errors.As(err, &apiErr) || apiErr.ErrorCode != 10093 { t.Fatalf("Expected API error 10093; got: %v", err) }