From 18e7dbc7d7a5f44f10809dc0bc0e41195fda7051 Mon Sep 17 00:00:00 2001 From: Patrik Date: Fri, 13 Nov 2026 11:13:24 +0100 Subject: [PATCH] fix: allow snake_case metadata keys in error catalogs Accept underscores and hyphens in metadata keys while preserving existing camelCase keys and rejecting invalid characters. Co-authored-by: Codex --- herodotpb/herodot.pb.go | 2 +- herodotpb/proto/ory/herodot/v1/herodot.proto | 2 +- v2/cmd/protoc-gen-herodot/catalog.go | 4 ++-- v2/cmd/protoc-gen-herodot/main_test.go | 23 +++++++++++++++++++- v2/options.go | 5 ++--- 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/herodotpb/herodot.pb.go b/herodotpb/herodot.pb.go index 3ba5c05..6f1eaa7 100644 --- a/herodotpb/herodot.pb.go +++ b/herodotpb/herodot.pb.go @@ -273,7 +273,7 @@ type ErrorSpec struct { // period, e.g. "The self-service flow has expired. Start a new flow." // Values that vary go into metadata, explanations into the comment. Message string `protobuf:"bytes,2,opt,name=message,proto3" json:"message,omitempty"` - // The documented ErrorInfo.metadata keys, in lowerCamelCase. + // The documented ErrorInfo.metadata keys, matching [a-z][a-zA-Z0-9_-]*. MetadataKeys []string `protobuf:"bytes,3,rep,name=metadata_keys,json=metadataKeys,proto3" json:"metadata_keys,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/herodotpb/proto/ory/herodot/v1/herodot.proto b/herodotpb/proto/ory/herodot/v1/herodot.proto index f7a4556..d7acbf1 100644 --- a/herodotpb/proto/ory/herodot/v1/herodot.proto +++ b/herodotpb/proto/ory/herodot/v1/herodot.proto @@ -62,7 +62,7 @@ message ErrorSpec { // period, e.g. "The self-service flow has expired. Start a new flow." // Values that vary go into metadata, explanations into the comment. string message = 2; - // The documented ErrorInfo.metadata keys, in lowerCamelCase. + // The documented ErrorInfo.metadata keys, matching [a-z][a-zA-Z0-9_-]*. repeated string metadata_keys = 3; } diff --git a/v2/cmd/protoc-gen-herodot/catalog.go b/v2/cmd/protoc-gen-herodot/catalog.go index f31dc39..96e6e9f 100644 --- a/v2/cmd/protoc-gen-herodot/catalog.go +++ b/v2/cmd/protoc-gen-herodot/catalog.go @@ -22,7 +22,7 @@ import ( var ( domainPattern = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+$`) reasonPattern = regexp.MustCompile(`^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$`) - metadataKeyPattern = regexp.MustCompile(`^[a-z][a-zA-Z0-9]*$`) + metadataKeyPattern = regexp.MustCompile(`^[a-z][a-zA-Z0-9_-]*$`) ) const maxReasonLength = 63 @@ -157,7 +157,7 @@ func loadReason(domain string, v *protogen.EnumValue, codes *codeTable) (*reason keys := map[string]bool{} for _, k := range spec.GetMetadataKeys() { if !metadataKeyPattern.MatchString(k) { - return nil, fmt.Errorf("metadata key %q is not lowerCamelCase", k) + return nil, fmt.Errorf("invalid metadata key %q: must match %s", k, metadataKeyPattern) } if keys[k] { return nil, fmt.Errorf("duplicate metadata key %q", k) diff --git a/v2/cmd/protoc-gen-herodot/main_test.go b/v2/cmd/protoc-gen-herodot/main_test.go index 3597d0b..c9fd624 100644 --- a/v2/cmd/protoc-gen-herodot/main_test.go +++ b/v2/cmd/protoc-gen-herodot/main_test.go @@ -191,7 +191,7 @@ func TestValidation(t *testing.T) { {"message with surrounding whitespace", map[string]string{"a.proto": catalog("a", "a.com", `X = 1 [(ory.herodot.v1.value).reason = {code: NOT_FOUND, message: "Not found. "}];`)}, "single line"}, {"metadata key", map[string]string{"a.proto": catalog("a", "a.com", - `X = 1 [(ory.herodot.v1.value).reason = {code: NOT_FOUND, message: "Message.", metadata_keys: ["flow_id"]}];`)}, `metadata key "flow_id" is not lowerCamelCase`}, + `X = 1 [(ory.herodot.v1.value).reason = {code: NOT_FOUND, message: "Message.", metadata_keys: ["flow.id"]}];`)}, `invalid metadata key "flow.id"`}, {"duplicate metadata key", map[string]string{"a.proto": catalog("a", "a.com", `X = 1 [(ory.herodot.v1.value).reason = {code: NOT_FOUND, message: "Message.", metadata_keys: ["a", "a"]}];`)}, `duplicate metadata key "a"`}, {"duplicate reason across files", map[string]string{ @@ -326,3 +326,24 @@ func TestOptionImport(t *testing.T) { require.Len(t, response.File, 1) assert.Contains(t, response.File[0].GetContent(), `herodot.Define("keto.ory.com", "NAMESPACE_NOT_FOUND", herodot.CodeNotFound`) } + +func TestMetadataKeys(t *testing.T) { + t.Parallel() + for _, key := range []string{"flow_id", "flowId", "flow-id", "a", "limit_per_request", "http2Status"} { + t.Run(key, func(t *testing.T) { + t.Parallel() + source := catalog("a", "a.com", `X = 1 [(ory.herodot.v1.value).reason = {code: NOT_FOUND, message: "Message.", metadata_keys: ["`+key+`"]}];`) + out, err := run(t, "lang=json", false, compile(t, map[string]string{"a.proto": source}, "a.proto")...) + require.NoError(t, err) + assert.Contains(t, out["a.com.errors.json"], `"`+key+`"`) + }) + } + for _, key := range []string{"", "FlowId", "1flow", "_flow", "flow.id", "flow id", "flow/id", "flöw"} { + t.Run("invalid="+key, func(t *testing.T) { + t.Parallel() + source := catalog("a", "a.com", `X = 1 [(ory.herodot.v1.value).reason = {code: NOT_FOUND, message: "Message.", metadata_keys: ["`+key+`"]}];`) + _, err := run(t, "lang=json", false, compile(t, map[string]string{"a.proto": source}, "a.proto")...) + require.ErrorContains(t, err, "invalid metadata key") + }) + } +} diff --git a/v2/options.go b/v2/options.go index 8074ad0..b93b523 100644 --- a/v2/options.go +++ b/v2/options.go @@ -42,9 +42,8 @@ func Cause(err error) Option { return optionFunc(func(d *errorData) { d.cause = err }) } -// Metadata adds an ErrorInfo metadata entry. Keys are lowerCamelCase and -// documented in the catalog. A later entry with the same key replaces an -// earlier one. +// Metadata adds an ErrorInfo metadata entry. Keys are documented in the catalog. +// A later entry with the same key replaces the earlier one. func Metadata(key, value string) Detail { return metadataDetail{key: key, value: value} }