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.

Sign up for GitHub

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

Jump to bottom

richer api errors proposal #2168

Merged
merged 3 commits into from May 26, 2021
Merged

richer api errors proposal #2168

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0d391b0
richer api errors proposal
ripienaar Apr 30, 2021
ee9d10f
restore old error constants for backwards compat
ripienaar May 26, 2021
639d07b
typo fix, report highest number
ripienaar May 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 9 additions & 5 deletions doc/adr/0007-error-codes.md
View file
Open in desktop
Expand Up @@ -77,7 +77,8 @@ if err != nil {
```

If we have to do string interpolation of the error body, here the `JSStreamRestoreErrF` has the body
`"restore failed: {err}"`, the `NewT()` will simply use `strings.Replaces()` to create a new `ApiError` with the full string:
`"restore failed: {err}"`, the `NewT()` will simply use `strings.Replaces()` to create a new `ApiError` with the full string,
note this is a new instance of ApiError so normal compare of `err == ApiErrors[x]` won't match:

```go
err = doRestore()
Expand All @@ -86,20 +87,23 @@ if err != nil {
}
```

If we had to handle an error that may be an `ApiError` or a traditional go error we can use the `ErrOrNew` function,
this will look at the result from `lookupConsumer()`, if it's an `ApiError` that error will be set else a `JSConsumerNotFoundErr`
will be created. Essentially the `lookupConsumer()` would return a `JSStreamNotFoundErr` if the stream does not exist else
If we had to handle an error that may be an `ApiError` or a traditional go error we can use the `ErrOr` function,
this will look at the result from `lookupConsumer()`, if it's an `ApiError` that error will be set else `JSConsumerNotFoundErr` be
returned. Essentially the `lookupConsumer()` would return a `JSStreamNotFoundErr` if the stream does not exist else
a `JSConsumerNotFoundErr` or go error on I/O failure for example.

```go
var resp = JSApiConsumerCreateResponse{ApiResponse: ApiResponse{Type: JSApiStreamCreateResponseType}}

_, err = lookupConsumer(stream, consumer)
if err != nil {
resp.Error = ApiErrors[JSConsumerNotFoundErr].ErrOrNew(err)
resp.Error = ApiErrors[JSConsumerNotFoundErr].ErrOr(err)
}
```

While the `ErrOr` function returns the `ApiErrors` pointer exactly - meaning `err == ApiErrors[x]`, the counterpart
`ErrOrNewT` will create a new instance.

### Testing Errors

Should you need to determine if a error is of a specific kind (error code) this can be done using the `IsNatsErr()` function:
Expand Down
40 changes: 29 additions & 11 deletions server/errors.json
View file
Open in desktop
Expand Up @@ -63,7 +63,8 @@
"constant": "JSInsufficientResourcesErr",
"code": 503,
"error_code": 10023,
"description": "insufficient resources"
"description": "insufficient resources",
"deprecates": "ErrJetStreamResourcesExceeded"
},
{
"constant": "JSMirrorMaxMessageSizeTooBigErr",
Expand Down Expand Up @@ -187,9 +188,17 @@
{
"constant": "JSNotEnabledErr",
"code": 503,
"error_code": 10076,
ripienaar marked this conversation as resolved.
Show resolved Hide resolved
"description": "JetStream not enabled",
"help": "This error indicates that JetStream is not enabled at a global level",
"deprectes": "ErrJetStreamNotEnabled"
ripienaar marked this conversation as resolved.
Show resolved Hide resolved
},
{
"constant": "JSNotEnabledForAccountErr",
"code": 503,
"error_code": 10039,
"description": "JetStream not enabled for account",
"help": "This error indicates that JetStream is not enabled either at a global level or at global and account level"
"help": "This error indicates that JetStream is not enabled for an account account level"
},
{
"constant": "JSSequenceNotFoundErrF",
Expand Down Expand Up @@ -225,7 +234,8 @@
"constant": "JSStorageResourcesExceededErr",
"code": 500,
"error_code": 10047,
"description": "insufficient storage resources available"
"description": "insufficient storage resources available",
"deprecates": "ErrStorageResourcesExceeded"
},
{
"constant": "JSStreamMismatchErr",
Expand Down Expand Up @@ -256,7 +266,8 @@
"constant": "JSStreamNameExistErr",
"code": 400,
"error_code": 10058,
"description": "stream name already in use"
"description": "stream name already in use",
"deprecates": "ErrJetStreamStreamAlreadyUsed"
},
{
"constant": "JSClusterTagsErr",
Expand Down Expand Up @@ -313,19 +324,22 @@
"constant": "JSClusterNotAssignedErr",
"code": 500,
"error_code": 10007,
"description": "JetStream cluster not assigned to this server"
"description": "JetStream cluster not assigned to this server",
"deprecates": "ErrJetStreamNotAssigned"
},
{
"constant": "JSClusterNotLeaderErr",
"code": 500,
"error_code": 10009,
"description": "JetStream cluster can not handle request"
"description": "JetStream cluster can not handle request",
"deprecates": "ErrJetStreamNotLeader"
},
{
"constant": "JSConsumerNameExistErr",
"code": 400,
"error_code": 10013,
"description": "consumer name already in use"
"description": "consumer name already in use",
"deprecates": "ErrJetStreamConsumerAlreadyUsed"
},
{
"constant": "JSMirrorWithSourcesErr",
Expand All @@ -337,7 +351,8 @@
"constant": "JSStreamNotFoundErr",
"code": 404,
"error_code": 10059,
"description": "stream not found"
"description": "stream not found",
"deprecates": "ErrJetStreamStreamNotFound"
},
{
"constant": "JSClusterRequiredErr",
Expand Down Expand Up @@ -380,7 +395,8 @@
"constant": "JSClusterNotActiveErr",
"code": 500,
"error_code": 10006,
"description": "JetStream not in clustered mode"
"description": "JetStream not in clustered mode",
"deprecates": "ErrJetStreamNotClustered"
},
{
"constant": "JSConsumerDurableNameNotMatchSubjectErr",
Expand All @@ -392,7 +408,8 @@
"constant": "JSMemoryResourcesExceededErr",
"code": 500,
"error_code": 10028,
"description": "insufficient memory resources available"
"description": "insufficient memory resources available",
"deprecates": "ErrMemoryResourcesExceeded"
},
{
"constant": "JSMirrorWithSubjectFiltersErr",
Expand Down Expand Up @@ -442,7 +459,8 @@
"constant": "JSStreamReplicasNotSupportedErr",
"code": 500,
"error_code": 10074,
"description": "replicas > 1 not supported in non-clustered mode"
"description": "replicas > 1 not supported in non-clustered mode",
"deprecates": "ErrReplicasNotSupported"
},
{
"constant": "JSStreamMsgDeleteFailedF",
Expand Down
10 changes: 9 additions & 1 deletion server/errors_gen.go
View file
Open in desktop
Expand Up @@ -33,7 +33,14 @@ var (
ApiErrors = map[ErrorIdentifier]*ApiError{
{{- range $i, $error := . }}
{{ .Constant }}: {Code: {{ .Code }},ErrCode: {{ .ErrCode }},Description: {{ .Description | printf "%q" }},{{- if .Help }}Help: {{ .Help | printf "%q" }},{{- end }}{{- if .URL }}URL: {{ .URL | printf "%q" }},{{- end }} },{{- end }}
}
}

{{- range $i, $error := . }}
{{- if .Deprecates }}
// {{ .Deprecates }} Deprecated by {{ .Constant }} ApiError, use IsNatsError() for comparisons
{{ .Deprecates }} = ApiErrors[{{ .Constant }}]
{{- end }}
{{- end }}
)
`

Expand All @@ -52,6 +59,7 @@ type Errors struct {
Comment string `json:"comment"`
Help string `json:"help"`
URL string `json:"url"`
Deprecates string `json:"deprecates"`
}

func goFmt(file string) error {
Expand Down
16 changes: 8 additions & 8 deletions server/jetstream.go
View file
Open in desktop
Expand Up @@ -775,7 +775,7 @@ func (s *Server) JetStreamNumAccounts() int {
func (s *Server) JetStreamReservedResources() (int64, int64, error) {
js := s.getJetStream()
if js == nil {
return -1, -1, ApiErrors[JSNotEnabledErr]
return -1, -1, ApiErrors[JSNotEnabledForAccountErr]
}
js.mu.RLock()
defer js.mu.RUnlock()
Expand Down Expand Up @@ -1107,7 +1107,7 @@ func (a *Account) lookupStream(name string) (*stream, error) {
a.mu.RUnlock()

if jsa == nil {
return nil, ApiErrors[JSNotEnabledErr]
return nil, ApiErrors[JSNotEnabledForAccountErr]
}
jsa.mu.Lock()
defer jsa.mu.Unlock()
Expand All @@ -1134,7 +1134,7 @@ func (a *Account) UpdateJetStreamLimits(limits *JetStreamAccountLimits) error {
return ApiErrors[JSNotEnabledErr]
}
if jsa == nil {
return ApiErrors[JSNotEnabledErr]
return ApiErrors[JSNotEnabledForAccountErr]
}

if limits == nil {
Expand Down Expand Up @@ -1223,7 +1223,7 @@ func (a *Account) DisableJetStream() error {

js := s.getJetStream()
if js == nil {
return ApiErrors[JSNotEnabledErr]
return ApiErrors[JSNotEnabledForAccountErr]
}

// Remove service imports.
Expand All @@ -1248,7 +1248,7 @@ func (a *Account) removeJetStream() error {

js := s.getJetStream()
if js == nil {
return ApiErrors[JSNotEnabledErr]
return ApiErrors[JSNotEnabledForAccountErr]
}

return js.disableJetStream(js.lookupAccount(a))
Expand All @@ -1257,7 +1257,7 @@ func (a *Account) removeJetStream() error {
// Disable JetStream for the account.
func (js *jetStream) disableJetStream(jsa *jsAccount) error {
if jsa == nil || jsa.account == nil {
return ApiErrors[JSNotEnabledErr]
return ApiErrors[JSNotEnabledForAccountErr]
}

js.mu.Lock()
Expand Down Expand Up @@ -1664,7 +1664,7 @@ func (a *Account) checkForJetStream() (*Server, *jsAccount, error) {
a.mu.RUnlock()

if s == nil || jsa == nil {
return nil, nil, ApiErrors[JSNotEnabledErr]
return nil, nil, ApiErrors[JSNotEnabledForAccountErr]
}

return s, jsa, nil
Expand Down Expand Up @@ -1875,7 +1875,7 @@ func (t *streamTemplate) delete() error {
t.mu.Unlock()

if jsa == nil {
return ApiErrors[JSNotEnabledErr]
return ApiErrors[JSNotEnabledForAccountErr]
}

jsa.mu.Lock()
Expand Down