search

grpc-ecosystem / grpc-httpjson-transcoding

Transcoding to provide HTTP/JSON interface for gRPC Service
Apache License 2.0
164 stars 35 forks source link

Allow detecting duplicate methods at build time #88

Closed euroelessar closed 1 year ago

euroelessar commented 1 year ago

Currently PathMatchBuilder does silently ignore duplicates at build time and fails lookups at request time.

Is it acceptable to add an option (or change the existing behavior) to fail method registration instead?

nareddyt commented 1 year ago

Good find, confirmed lookup fails silently when duplicate binding is registered. Not sure if there is historical context we are missing for this design decision.

Is it acceptable to add an option (or change the existing behavior) to fail method registration instead?

Let's add an option (similar to https://github.com/grpc-ecosystem/grpc-httpjson-transcoding/pull/66) for safety. Are you using this through Envoy gRPC transcoding filter? We can decide if it's safe to default the option to true in Envoy later.

FYI right now we are tight on dev cycles, so it will take some time for me to fix it. Feel free to make the PR yourself if you have time.

euroelessar commented 1 year ago

Sounds good, I'll send a pr within next week or so. We're using this library both through envoy and directly, so yeah, we can discuss enabling this option by default in envoy issue tracker separately.