From a0fcf0bb6539d678dbf57f44514575ba1112056a Mon Sep 17 00:00:00 2001 From: "R.I.Pienaar" Date: Fri, 18 Jun 2021 20:11:09 +0200 Subject: [PATCH] further tagged error confusion cleanups Signed-off-by: R.I.Pienaar --- server/jetstream_api.go | 16 ++++++++-------- server/jetstream_cluster.go | 6 +++--- server/jetstream_errors_test.go | 31 ++++++++++++++++++------------- 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/server/jetstream_api.go b/server/jetstream_api.go index fc9f82d9..174bcdfb 100644 --- a/server/jetstream_api.go +++ b/server/jetstream_api.go @@ -1351,7 +1351,7 @@ func (s *Server) jsStreamUpdateRequest(sub *subscription, c *client, subject, re mset, err := acc.lookupStream(streamName) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } @@ -2199,13 +2199,13 @@ func (s *Server) jsStreamDeleteRequest(sub *subscription, c *client, subject, re mset, err := acc.lookupStream(stream) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } if err := mset.delete(); err != nil { - resp.Error = ApiErrors[JSStreamDeleteErrF].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamDeleteErrF].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } @@ -3101,7 +3101,7 @@ func (s *Server) jsConsumerCreate(sub *subscription, c *client, subject, reply s stream, err := acc.lookupStream(req.Stream) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } @@ -3208,7 +3208,7 @@ func (s *Server) jsConsumerNamesRequest(sub *subscription, c *client, subject, r } else { mset, err := acc.lookupStream(streamName) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } @@ -3302,7 +3302,7 @@ func (s *Server) jsConsumerListRequest(sub *subscription, c *client, subject, re mset, err := acc.lookupStream(streamName) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } @@ -3425,7 +3425,7 @@ func (s *Server) jsConsumerInfoRequest(sub *subscription, c *client, subject, re mset, err := acc.lookupStream(streamName) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } @@ -3492,7 +3492,7 @@ func (s *Server) jsConsumerDeleteRequest(sub *subscription, c *client, subject, mset, err := acc.lookupStream(stream) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp)) return } diff --git a/server/jetstream_cluster.go b/server/jetstream_cluster.go index e4bc13e7..050fd81d 100644 --- a/server/jetstream_cluster.go +++ b/server/jetstream_cluster.go @@ -2331,7 +2331,7 @@ func (js *jetStream) processClusterDeleteStream(sa *streamAssignment, isMember, // Go ahead and delete the stream. mset, err := acc.lookupStream(sa.Config.Name) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) } else if mset != nil { err = mset.stop(true, wasLeader) } @@ -2614,7 +2614,7 @@ func (js *jetStream) processClusterDeleteConsumer(ca *consumerAssignment, isMemb // Go ahead and delete the consumer. mset, err := acc.lookupStream(ca.Stream) if err != nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) } else if mset != nil { if o := mset.lookupConsumer(ca.Name); o != nil { err = o.stopWithFlags(true, true, wasLeader) @@ -2633,7 +2633,7 @@ func (js *jetStream) processClusterDeleteConsumer(ca *consumerAssignment, isMemb if err != nil { if resp.Error == nil { - resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOrNewT(err, "{err}", err) + resp.Error = ApiErrors[JSStreamNotFoundErr].ErrOr(err) } s.sendAPIErrResponse(ca.Client, acc, ca.Subject, ca.Reply, _EMPTY_, s.jsonResponse(resp)) } else { diff --git a/server/jetstream_errors_test.go b/server/jetstream_errors_test.go index 8171de92..95ec8e79 100644 --- a/server/jetstream_errors_test.go +++ b/server/jetstream_errors_test.go @@ -47,7 +47,7 @@ func TestApiError_Error(t *testing.T) { } } -func TestApiError_NewF(t *testing.T) { +func TestApiError_NewT(t *testing.T) { ne := ApiErrors[JSRestoreSubscribeFailedErrF].NewT("{subject}", "the.subject", "{err}", errors.New("failed error")) if ne.Description != "JetStream unable to subscribe to restore snapshot the.subject: failed error" { t.Fatalf("Expected 'JetStream unable to subscribe to restore snapshot the.subject: failed error' got %q", ne.Description) @@ -56,9 +56,18 @@ func TestApiError_NewF(t *testing.T) { if ne == ApiErrors[JSRestoreSubscribeFailedErrF] { t.Fatalf("Expected a new instance") } + + aerr := ApiError{ + Code: 999, + Description: "thing {string} failed on attempt {int} after {duration} with {float}: {err}", + } + + if ne := aerr.NewT("{float}", 1.1, "{err}", fmt.Errorf("simulated error"), "{string}", "hello world", "{int}", 10, "{duration}", 456*time.Millisecond); ne.Description != "thing hello world failed on attempt 10 after 456ms with 1.1: simulated error" { + t.Fatalf("Expected formatted error, got: %q", ne.Description) + } } -func TestApiError_ErrOrNewF(t *testing.T) { +func TestApiError_ErrOrNewT(t *testing.T) { if ne := ApiErrors[JSStreamRestoreErrF].ErrOrNewT(ApiErrors[JSNotEnabledForAccountErr], "{err}", errors.New("failed error")); !IsNatsErr(ne, JSNotEnabledForAccountErr) { t.Fatalf("Expected JSNotEnabledForAccountErr got %s", ne) } @@ -70,6 +79,13 @@ func TestApiError_ErrOrNewF(t *testing.T) { if ne := ApiErrors[JSStreamRestoreErrF].ErrOrNewT(errors.New("other error"), "{err}", errors.New("failed error")); !IsNatsErr(ne, JSStreamRestoreErrF) { t.Fatalf("Expected JSStreamRestoreErrF got %s", ne) } + + // ensure that mistakenly passing a non tagged error to this function is harmless + if ne := ApiErrors[JSStreamNotFoundErr].ErrOrNewT(errors.New("other error"), "{err}", errors.New("failed error")); !IsNatsErr(ne, JSStreamNotFoundErr) { + t.Fatalf("Expected JSStreamNotFoundErr got %s", ne) + } else if ne.Description != ApiErrors[JSStreamNotFoundErr].Description { + t.Fatalf("Expected JSStreamNotFoundErr description got: %s", ne.Description) + } } func TestApiError_ErrOr(t *testing.T) { @@ -85,14 +101,3 @@ func TestApiError_ErrOr(t *testing.T) { t.Fatalf("Expected JSPeerRemapErr got %s", ne) } } - -func TestApiError_NewT(t *testing.T) { - aerr := ApiError{ - Code: 999, - Description: "thing {string} failed on attempt {int} after {duration} with {float}: {err}", - } - - if ne := aerr.NewT("{float}", 1.1, "{err}", fmt.Errorf("simulated error"), "{string}", "hello world", "{int}", 10, "{duration}", 456*time.Millisecond); ne.Description != "thing hello world failed on attempt 10 after 456ms with 1.1: simulated error" { - t.Fatalf("Expected formatted error, got: %q", ne.Description) - } -}