twitchtv / twirp

A simple RPC framework with protobuf service definitions
https://twitchtv.github.io/twirp/docs/intro.html
Apache License 2.0
7.2k stars 326 forks source link

Twirp handler only works with mux.Handler, doesn't work with chi.Handler #386

Closed skhreshefsharvit closed 10 months ago

skhreshefsharvit commented 1 year ago

I try to run a server that handles both REST and RPC, and with Golang's Chi it just doesn't work for some reason, even though Chi is compatible with the standard library, just like http.NewServeMux().

example: `func NewChiHandler(usecase exclusion.ExclusionUseCase) (http.Handler, error) { handlerFunctions := &ChiHandler{ Usecase: usecase, }

swaggerSpec, err := api.GetSwagger()
if err != nil {
    return nil, err
}

r := chi.NewRouter()

r.Use(chiMiddleware.Logger)
r.Use(chiMiddleware.RequestID)
r.Use(chiMiddleware.RealIP)
r.Use(chiMiddleware.Recoverer)
r.Use(middleware.OapiRequestValidator(swaggerSpec))

strictHandler := api.NewStrictHandler(handlerFunctions, nil)

twirpHandler := api.NewExclusionServer(handlerFunctions)
r.Handle(twirpHandler.PathPrefix(), r)
r.Handle("/", api.HandlerFromMux(strictHandler, r))

return r, nil

} `

`func main() { // create an usecase implementation. we only need to pass a repository object. usecaseImplementation := exclusion.NewExclusionUseCaseImplementation()

serverHandler, err := handler.NewChiHandler(usecaseImplementation)
if err != nil {
    panic("could not initialize handler with error: " + err.Error())
}

http.ListenAndServe(":8000", serverHandler)

} `

when I execute the this, my requests keep getting 404 - "twirp error bad_route: Error from intermediary with HTTP status code 404 "Not Found"

However, when I switch to http.ServeMux, it works perfectly. I guess it has something to do with the way Handle works for both Chi and ServeMux. Any idea?

swiftyspiffy commented 12 months ago

Hi, apologies for the late response.

My colleagues and I are not super familiar with Chi. There is some amount of complexity added when using Chi based on the code you've shown and we don't really have a way of testing for Chi compatibility at the moment. Are you able to create a public Go playground that demonstrates the issue? If you simplify your Chi usage with Twirp to as simple as possible, does the issue still show up?

In other words, I'm unsure if this is a Twirp or Chi problem (or a combination of the two), and without knowledge of Chi or a playground demonstration of the problem, I'm not sure we can help.

swiftyspiffy commented 10 months ago

Closing this, feel free to leave additional comments if you have anything to ad.

mattrandallbecker commented 10 months ago

FYI, this issue comes up on top when you Google "twirp chi". Figured I'd leave how I got it working. I got a tip by diving into older issues, specifically this one: https://github.com/twitchtv/twirp/issues/252

Here is what's working for me:

import (
  api "path/to/your/twirp/generated/go/package"
  // ...other imports
)

func (s *Server) Routes(ctx context.Context) (http.Handler, error) {
    r := chi.NewRouter()
    r.Use(middleware.Logger)

        // s.handler should implement all of the function prototypes from NewServiceNameServer
        r.Mount("/twirp", api.NewServiceNameServer(s.handler))
        // ...rest of your routes