Making the reasons a string would make for more readable logs

[sazzer]

Currently the cacheobject.Reason type is an int. This works well enough for the library itself, but it's a shame that it doesn't log very well.

For example, right now if parsing a no-store header I get the following log message:

{"level":"warn","reasons":[9],"time":"2021-08-30T17:08:41+01:00","message":"JWK keys are not cacheable"}

The "reasons":[9] part in particular is not very useful.

If the cacheobject.Reason type was instead a human readable string then the log messages here would be much more useful, without actually compromising anything about using the library.

[sazzer]

For example, changing it to:

type Reason string

const (
.....
	// The request included an Cache-Control: no-store header
	ReasonRequestNoStore = "no-store"
.....
)

would make the library work exactly the same, but the log outputs would then be:

{"level":"warn","reasons":["no-store"],"time":"2021-08-30T17:08:41+01:00","message":"JWK keys are not cacheable"}

which is much more useful.

[pquerna]

I agree. this could be a good change. is there a way to do this without breaking semver?

[sazzer]

I don't know Go well enough to say for certain, but I think that the fact you've got a custom type covers you here. From the point of view of the code using this library, it's still just a cacheobject.Reason type with opaque values. It's just that those values are now different.

The problem - and this is where my knowledge of Go and Semver lets me down - is if anyone is ever assuming that the values are int and can be safely treated as such. For example with %d operators in fmt.Printf. Now, arguably they are breaking the contract with the library in those cases - after all, the type isn't int but rather cacheobject.Reason and so it shouldn't be treated as if it were an int - but that doesn't mean that nobody ever does that.