Skip to content

Commit

Permalink
Merge pull request #3865 from nats-io/jarema/improve-js-error-messages
Browse files Browse the repository at this point in the history
Make JetStream errors naming consistent
  • Loading branch information
Jarema committed Feb 13, 2023
2 parents 0551c2f + 45deb3e commit ba6d4ff
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 48 deletions.
9 changes: 8 additions & 1 deletion server/consumer.go
Expand Up @@ -495,6 +495,13 @@ func checkConsumerCfg(
}
}

// Do not allow specifying both FilterSubject and FilterSubjects,
// as that's probably unintentional without any difference from passing
// all filters in FilterSubjects.
if config.FilterSubject != _EMPTY_ && len(config.FilterSubjects) > 0 {
return NewJSConsumerDuplicateFilterSubjectsError()
}

if config.FilterSubject != _EMPTY_ && !IsValidSubject(config.FilterSubject) {
return NewJSStreamInvalidConfigError(ErrBadSubject)
}
Expand All @@ -514,7 +521,7 @@ func checkConsumerCfg(
}
for inner, ssubject := range subjectFilters {
if inner != outer && subjectIsSubsetMatch(subject, ssubject) {
return NewJsConsumerOverlappingSubjectFiltersError()
return NewJSConsumerOverlappingSubjectFiltersError()
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions server/errors.json
Expand Up @@ -1340,7 +1340,7 @@
"deprecates": ""
},
{
"constant": "JsConsumerDuplicateFilterSubjects",
"constant": "JSConsumerDuplicateFilterSubjects",
"code": 400,
"error_code": 10136,
"description": "consumer cannot have both FilterSubject and FilterSubjects specified",
Expand All @@ -1350,7 +1350,7 @@
"deprecates": ""
},
{
"constant": "JsConsumerMultipleFiltersNotAllowed",
"constant": "JSConsumerMultipleFiltersNotAllowed",
"code": 400,
"error_code": 10137,
"description": "consumer with multiple subject filters cannot use subject based API",
Expand All @@ -1360,7 +1360,7 @@
"deprecates": ""
},
{
"constant": "JsConsumerOverlappingSubjectFilters",
"constant": "JSConsumerOverlappingSubjectFilters",
"code": 400,
"error_code": 10138,
"description": "consumer subject filters cannot overlap",
Expand Down
2 changes: 1 addition & 1 deletion server/jetstream_api.go
Expand Up @@ -3820,7 +3820,7 @@ func (s *Server) jsConsumerCreateRequest(sub *subscription, c *client, a *Accoun

// in case of multiple filters provided, error if new API is used.
if filteredSubject != _EMPTY_ && len(req.Config.FilterSubjects) != 0 {
resp.Error = NewJsConsumerMultipleFiltersNotAllowedError()
resp.Error = NewJSConsumerMultipleFiltersNotAllowedError()
s.sendAPIErrResponse(ci, acc, subject, reply, string(msg), s.jsonResponse(&resp))
return
}
Expand Down
84 changes: 42 additions & 42 deletions server/jetstream_errors_generated.go
Expand Up @@ -74,6 +74,9 @@ const (
// JSConsumerDirectRequiresPushErr consumer direct requires a push based consumer
JSConsumerDirectRequiresPushErr ErrorIdentifier = 10090

// JSConsumerDuplicateFilterSubjects consumer cannot have both FilterSubject and FilterSubjects specified
JSConsumerDuplicateFilterSubjects ErrorIdentifier = 10136

// JSConsumerDurableNameNotInSubjectErr consumer expected to be durable but no durable name set in subject
JSConsumerDurableNameNotInSubjectErr ErrorIdentifier = 10016

Expand Down Expand Up @@ -137,6 +140,9 @@ const (
// JSConsumerMetadataLengthErrF consumer metadata exceeds maximum size of {limit}
JSConsumerMetadataLengthErrF ErrorIdentifier = 10135

// JSConsumerMultipleFiltersNotAllowed consumer with multiple subject filters cannot use subject based API
JSConsumerMultipleFiltersNotAllowed ErrorIdentifier = 10137

// JSConsumerNameContainsPathSeparatorsErr Consumer name can not contain path separators
JSConsumerNameContainsPathSeparatorsErr ErrorIdentifier = 10127

Expand All @@ -155,6 +161,9 @@ const (
// JSConsumerOnMappedErr consumer direct on a mapped consumer
JSConsumerOnMappedErr ErrorIdentifier = 10092

// JSConsumerOverlappingSubjectFilters consumer subject filters cannot overlap
JSConsumerOverlappingSubjectFilters ErrorIdentifier = 10138

// JSConsumerPullNotDurableErr consumer in pull mode requires a durable name
JSConsumerPullNotDurableErr ErrorIdentifier = 10085

Expand Down Expand Up @@ -409,15 +418,6 @@ const (

// JSTemplateNameNotMatchSubjectErr template name in subject does not match request
JSTemplateNameNotMatchSubjectErr ErrorIdentifier = 10073

// JsConsumerDuplicateFilterSubjects consumer cannot have both FilterSubject and FilterSubjects specified
JsConsumerDuplicateFilterSubjects ErrorIdentifier = 10136

// JsConsumerMultipleFiltersNotAllowed consumer with multiple subject filters cannot use subject based API
JsConsumerMultipleFiltersNotAllowed ErrorIdentifier = 10137

// JsConsumerOverlappingSubjectFilters consumer subject filters cannot overlap
JsConsumerOverlappingSubjectFilters ErrorIdentifier = 10138
)

var (
Expand Down Expand Up @@ -445,6 +445,7 @@ var (
JSConsumerDescriptionTooLongErrF: {Code: 400, ErrCode: 10107, Description: "consumer description is too long, maximum allowed is {max}"},
JSConsumerDirectRequiresEphemeralErr: {Code: 400, ErrCode: 10091, Description: "consumer direct requires an ephemeral consumer"},
JSConsumerDirectRequiresPushErr: {Code: 400, ErrCode: 10090, Description: "consumer direct requires a push based consumer"},
JSConsumerDuplicateFilterSubjects: {Code: 400, ErrCode: 10136, Description: "consumer cannot have both FilterSubject and FilterSubjects specified"},
JSConsumerDurableNameNotInSubjectErr: {Code: 400, ErrCode: 10016, Description: "consumer expected to be durable but no durable name set in subject"},
JSConsumerDurableNameNotMatchSubjectErr: {Code: 400, ErrCode: 10017, Description: "consumer name in subject does not match durable name in request"},
JSConsumerDurableNameNotSetErr: {Code: 400, ErrCode: 10018, Description: "consumer expected to be durable but a durable name was not set"},
Expand All @@ -466,12 +467,14 @@ var (
JSConsumerMaxRequestExpiresToSmall: {Code: 400, ErrCode: 10115, Description: "consumer max request expires needs to be >= 1ms"},
JSConsumerMaxWaitingNegativeErr: {Code: 400, ErrCode: 10087, Description: "consumer max waiting needs to be positive"},
JSConsumerMetadataLengthErrF: {Code: 400, ErrCode: 10135, Description: "consumer metadata exceeds maximum size of {limit}"},
JSConsumerMultipleFiltersNotAllowed: {Code: 400, ErrCode: 10137, Description: "consumer with multiple subject filters cannot use subject based API"},
JSConsumerNameContainsPathSeparatorsErr: {Code: 400, ErrCode: 10127, Description: "Consumer name can not contain path separators"},
JSConsumerNameExistErr: {Code: 400, ErrCode: 10013, Description: "consumer name already in use"},
JSConsumerNameTooLongErrF: {Code: 400, ErrCode: 10102, Description: "consumer name is too long, maximum allowed is {max}"},
JSConsumerNotFoundErr: {Code: 404, ErrCode: 10014, Description: "consumer not found"},
JSConsumerOfflineErr: {Code: 500, ErrCode: 10119, Description: "consumer is offline"},
JSConsumerOnMappedErr: {Code: 400, ErrCode: 10092, Description: "consumer direct on a mapped consumer"},
JSConsumerOverlappingSubjectFilters: {Code: 400, ErrCode: 10138, Description: "consumer subject filters cannot overlap"},
JSConsumerPullNotDurableErr: {Code: 400, ErrCode: 10085, Description: "consumer in pull mode requires a durable name"},
JSConsumerPullRequiresAckErr: {Code: 400, ErrCode: 10084, Description: "consumer in pull mode requires ack policy"},
JSConsumerPullWithRateLimitErr: {Code: 400, ErrCode: 10086, Description: "consumer in pull mode can not have rate limit set"},
Expand Down Expand Up @@ -557,9 +560,6 @@ var (
JSStreamWrongLastSequenceErrF: {Code: 400, ErrCode: 10071, Description: "wrong last sequence: {seq}"},
JSTempStorageFailedErr: {Code: 500, ErrCode: 10072, Description: "JetStream unable to open temp storage for restore"},
JSTemplateNameNotMatchSubjectErr: {Code: 400, ErrCode: 10073, Description: "template name in subject does not match request"},
JsConsumerDuplicateFilterSubjects: {Code: 400, ErrCode: 10136, Description: "consumer cannot have both FilterSubject and FilterSubjects specified"},
JsConsumerMultipleFiltersNotAllowed: {Code: 400, ErrCode: 10137, Description: "consumer with multiple subject filters cannot use subject based API"},
JsConsumerOverlappingSubjectFilters: {Code: 400, ErrCode: 10138, Description: "consumer subject filters cannot overlap"},
}
// ErrJetStreamNotClustered Deprecated by JSClusterNotActiveErr ApiError, use IsNatsError() for comparisons
ErrJetStreamNotClustered = ApiErrors[JSClusterNotActiveErr]
Expand Down Expand Up @@ -833,6 +833,16 @@ func NewJSConsumerDirectRequiresPushError(opts ...ErrorOption) *ApiError {
return ApiErrors[JSConsumerDirectRequiresPushErr]
}

// NewJSConsumerDuplicateFilterSubjectsError creates a new JSConsumerDuplicateFilterSubjects error: "consumer cannot have both FilterSubject and FilterSubjects specified"
func NewJSConsumerDuplicateFilterSubjectsError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
if ae, ok := eopts.err.(*ApiError); ok {
return ae
}

return ApiErrors[JSConsumerDuplicateFilterSubjects]
}

// NewJSConsumerDurableNameNotInSubjectError creates a new JSConsumerDurableNameNotInSubjectErr error: "consumer expected to be durable but no durable name set in subject"
func NewJSConsumerDurableNameNotInSubjectError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
Expand Down Expand Up @@ -1073,6 +1083,16 @@ func NewJSConsumerMetadataLengthError(limit interface{}, opts ...ErrorOption) *A
}
}

// NewJSConsumerMultipleFiltersNotAllowedError creates a new JSConsumerMultipleFiltersNotAllowed error: "consumer with multiple subject filters cannot use subject based API"
func NewJSConsumerMultipleFiltersNotAllowedError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
if ae, ok := eopts.err.(*ApiError); ok {
return ae
}

return ApiErrors[JSConsumerMultipleFiltersNotAllowed]
}

// NewJSConsumerNameContainsPathSeparatorsError creates a new JSConsumerNameContainsPathSeparatorsErr error: "Consumer name can not contain path separators"
func NewJSConsumerNameContainsPathSeparatorsError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
Expand Down Expand Up @@ -1139,6 +1159,16 @@ func NewJSConsumerOnMappedError(opts ...ErrorOption) *ApiError {
return ApiErrors[JSConsumerOnMappedErr]
}

// NewJSConsumerOverlappingSubjectFiltersError creates a new JSConsumerOverlappingSubjectFilters error: "consumer subject filters cannot overlap"
func NewJSConsumerOverlappingSubjectFiltersError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
if ae, ok := eopts.err.(*ApiError); ok {
return ae
}

return ApiErrors[JSConsumerOverlappingSubjectFilters]
}

// NewJSConsumerPullNotDurableError creates a new JSConsumerPullNotDurableErr error: "consumer in pull mode requires a durable name"
func NewJSConsumerPullNotDurableError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
Expand Down Expand Up @@ -2150,33 +2180,3 @@ func NewJSTemplateNameNotMatchSubjectError(opts ...ErrorOption) *ApiError {

return ApiErrors[JSTemplateNameNotMatchSubjectErr]
}

// NewJsConsumerDuplicateFilterSubjectsError creates a new JsConsumerDuplicateFilterSubjects error: "consumer cannot have both FilterSubject and FilterSubjects specified"
func NewJsConsumerDuplicateFilterSubjectsError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
if ae, ok := eopts.err.(*ApiError); ok {
return ae
}

return ApiErrors[JsConsumerDuplicateFilterSubjects]
}

// NewJsConsumerMultipleFiltersNotAllowedError creates a new JsConsumerMultipleFiltersNotAllowed error: "consumer with multiple subject filters cannot use subject based API"
func NewJsConsumerMultipleFiltersNotAllowedError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
if ae, ok := eopts.err.(*ApiError); ok {
return ae
}

return ApiErrors[JsConsumerMultipleFiltersNotAllowed]
}

// NewJsConsumerOverlappingSubjectFiltersError creates a new JsConsumerOverlappingSubjectFilters error: "consumer subject filters cannot overlap"
func NewJsConsumerOverlappingSubjectFiltersError(opts ...ErrorOption) *ApiError {
eopts := parseOpts(opts)
if ae, ok := eopts.err.(*ApiError); ok {
return ae
}

return ApiErrors[JsConsumerOverlappingSubjectFilters]
}
27 changes: 26 additions & 1 deletion server/jetstream_test.go
Expand Up @@ -9387,7 +9387,7 @@ func TestJetStreamPubWithSyncPerf(t *testing.T) {

func TestJetStreamConsumerPerf(t *testing.T) {
// Comment out to run, holding place for now.
// t.SkipNow()
t.SkipNow()

s := RunBasicJetStreamServer(t)
defer s.Shutdown()
Expand Down Expand Up @@ -19228,6 +19228,31 @@ func TestJetStreamConsumerOverlappingSubjects(t *testing.T) {
}
}

func TestJetStreamBothFiltersSet(t *testing.T) {
s := RunBasicJetStreamServer(t)
if config := s.JetStreamConfig(); config != nil {
defer removeDir(t, config.StoreDir)
}
defer s.Shutdown()

nc, _ := jsClientConnect(t, s)
defer nc.Close()
acc := s.GlobalAccount()

_, err := acc.addStream(&StreamConfig{
Subjects: []string{"events.>"},
Name: "deliver",
})
require_NoError(t, err)

resp := createConsumer(t, nc, "deliver", ConsumerConfig{
FilterSubjects: []string{"events.one", "events.two"},
FilterSubject: "events.three",
Durable: "name",
})
require_True(t, resp.Error.ErrCode == 10136)
}

func TestJetStreamMultipleSubjectsPushBasic(t *testing.T) {
s := RunBasicJetStreamServer(t)
if config := s.JetStreamConfig(); config != nil {
Expand Down

0 comments on commit ba6d4ff

Please sign in to comment.