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.

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

If there is no package in the proto file, an unnecessary "." is added to the Path of the HTTP Handler. #597

Closed
dicenull opened this issue Sep 27, 2023 · 1 comment · Fixed by #601
Assignees

Comments

@dicenull
Copy link

Describe the bug

When there is no package in the proto file, an unnecessary "." is added to the Path of the HTTP Handler.
ServeHTTP will not be called because the path used in the branch and the path used in the implementation are generated differently.

To reproduce

  • coonnect-go v1.9.0

Reproducible with this branch.

  • Remove package specification from ping.proto.
  • Run buf generate.
  • ping.connect.go file is generated.
  • For the PingServiceName = "PingService" constant, the NewPingServiceHandler method will result in "/.PingService/".

Comment

service.Desc.ParentFile().Package() is empty string and . + service.Desc.Name().
I think this bug occurs because this reflectionName and service.Desc.FullName() do not match.

func reflectionName(service *protogen.Service) string {
  return fmt.Sprintf("%s.%s", service.Desc.ParentFile().Package(), service.Desc.Name())
}

This text was translated from Japanese by DeepL.

Original text

バグの説明

protoファイルにpackageがない場合、HTTP HandlerのPathに不要な"."が付与される。
分岐で使用するパスと、実装で使うパスが異なって生成されるためServeHTTPが呼ばれなくなってしまう。

再現方法

  • coonnect-go v1.9.0

このブランチで再現可能。

  • ping.proto のpackage指定を削除する。
  • buf generateを実行する。
  • ping.connect.goファイルが生成される。
  • PingServiceName = "PingService"定数に対して、NewPingServiceHandlerメソッドでは"/.PingService/"になっている。

コメント

service.Desc.ParentFile().Package() が空文字列になり、. + service.Desc.Name()となってしまう。
このreflectionNameservice.Desc.FullName()が一致しないため、このバグが発生していると思います。

func reflectionName(service *protogen.Service) string {
  return fmt.Sprintf("%s.%s", service.Desc.ParentFile().Package(), service.Desc.Name())
}

この文章は日本語をDeepLで翻訳した。

@emcfarlane
Copy link
Contributor

Hey dicenull, thanks for the detailed breakdown and report!

akshayjshah added a commit that referenced this issue Oct 3, 2023
Use the protobuf runtime's `FullName` rather than our own, bug-prone
implementation.

Fixes #597.

---------

Co-authored-by: Akshay Shah <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants