nats.go icon indicating copy to clipboard operation
nats.go copied to clipboard

incorrect error message when creating KV watches

Open critterjohnson opened this issue 1 year ago • 5 comments

Defect

Versions of nats.go and the nats-server if one was involved:

nats-server: v2.9.19 github.com/nats-io/nats.go v1.27.1

OS/Container environment:

MacOS 13.4.1 go version go1.20.5 darwin/arm64

Steps or code to reproduce the issue:

Start a JetStream enabled NATS server:

nats-server -js

Spawn a watch on an invalid key:

package main

import (
	"github.com/nats-io/nats.go"
)

func main() {
	nc, _ := nats.Connect(nats.DefaultURL)

	js, err := nc.JetStream()
	if err != nil {
		panic(err)
	}

	kv, err := js.KeyValue("bucket")
	if err != nil {
		panic(err)
	}

	_, err = kv.Watch("test..")
	if err != nil {
		panic(err)
	}
}

Expected result:

An error message similar to the one the CLI provides:

$ nats kv watch bucket test..
nats: error: nats: consumer filter subject is not a valid subset of the interest subjects

Actual result:

panic: nats: jetstream not enabled

critterjohnson avatar Jul 21 '23 18:07 critterjohnson

It's also worth noting that the nats CLI current main branch also shows the error:

nats: error: nats: jetstream not enabled

I can only get the error:

nats: error: nats: consumer filter subject is not a valid subset of the interest subjects

With nats CLI v0.0.35 uses nats.go version 1.19.0:

go install github.com/nats-io/natscli/[email protected]

mdawar avatar Jul 23 '23 14:07 mdawar

It is because the trailing “..” is an invalid subject and it causes the API subjects being used to also be invalid.

We should probably fail for such invalid sibjects earlier - but that’s why you get jetstream not enabled, the jetstream API can’t be reached.

ripienaar avatar Jul 23 '23 14:07 ripienaar

It is because the trailing “..” is an invalid subject and it causes the API subjects being used to also be invalid.

We should probably fail for such invalid sibjects earlier - but that’s why you get jetstream not enabled, the jetstream API can’t be reached.

Yes the Watch() method does not validate the keys pattern, the other KV methods use the keyValid() function to validate the key which is going to fail if in Watch() if the pattern contains wildcards.

mdawar avatar Jul 23 '23 14:07 mdawar

It is because the trailing “..” is an invalid subject and it causes the API subjects being used to also be invalid.

We should probably fail for such invalid sibjects earlier - but that’s why you get jetstream not enabled, the jetstream API can’t be reached.

I understand that - I found where it's coming from in the source code but I figured I should file an issue here.

FWIW, this came up when I was accidentally passing an empty string to a format string that constructed the key for me. The bad error message led me to believe that there was some kind of server-side error, and it took me a good half hour to figure out what was going on.

critterjohnson avatar Jul 24 '23 12:07 critterjohnson

I think it's an issue with the Go API design. I would prefer that if the NATS server is going to complain about subject formatting, then it should accept a Subject type instead of a string so it's harder to receive an invalid one.

For example, in the subject package, it would be possible to have a Parse(s string) (s Subject, err error) function that checks that the subject is valid using the same rules as the server. You'd use it with subject.Parse("my_subject").

It might have helper methods on it to build up valid subjects, wildcards etc.

a-h avatar Oct 20 '23 14:10 a-h