twitchtv / twirp Goto Github PK

The generated twirp servers include
import log "log"

They're used in error cases like this:

	if _, err = resp.Write(buf.Bytes()); err != nil {
		log.Printf("errored while writing response to client, but already sent response status code to 200: %s", err)
	}

It would be great if I could provide my own logger of some kind.

One proposed API would be to expose an optional function I could type assert for.

func (s *myServer) SetLogger(logger *log.Logger) {
  s.logger = logger
}

The client code could then call

x := NewMyServer(...)
x.(twirp.Logthingie).SetLogger(myOwnLogger)

If you're ambitious, you could have a specific twirp type logger for more strongly typed information

type TwirpLogger interface {
  ErrorWritingReponse(ctx context.Context, writeErr err)
}

My implementation could then extract the method name from the ctx.

Awesome framework!! 🥇 👍 👍 👍

Im new to RPC and i looked around all over and i didn't find what i was looking for so as a last resort im posting my issue here.. so please forgive me if it does not much sense.

Im having an issue with a simple .proto file i made for a hash generator API endpoint

syntax = "proto3";
package store.users.auth;

service Session {
    rpc Generate(Empty) returns (SessionResp);
  }
  
  message Empty {}
  
  message Session {
    string text = 1;
  }

This function generates a hash key that gets returned to the frontend and saved in a db that i use for another part of my app.
How can i trigger the API without sending any JSON ... this works fine now, the only think i need to do is to send a empty json object {}

and i get my expected result

{
    "key": "5f6ce9c6-4276-40aa-86e0-326134ce8a9b"
}

Otherwise i get this

{
    "code": "internal",
    "msg": "failed to parse request json: EOF",
    "meta": {
        "cause": "*store_users_auth.wrappedError"
    }
}

How can i fix it in the .proto file so that the generate function's request ( Empty ) will work without an empty JSON post

Im hoping there is a easy way to do this because i don't to mess around with the .pb.go or the .twirp.go files.

Please help a newb :)

I think we could improve Twirp's routing scheme. It has two problems right now: the /twirp/ prefix, and the package.service scheme (instead of package/service).

Some people have mentioned to me that they're hesitant to use Twirp because the routes are prefixed with /twirp, which is concerning to them for several reasons:

• Trademarks: some very large organizations don't want to take any legal risks and are concerned that "twirp" could become trademarked.
• Feels like advertising: This was mentioned in #48: putting "twirp" in all your routes feels like it's just supposed to pump Twirp's brand.
• Homophonous with "twerp": In some Very Serious settings (like government websites), it's not okay that "twirp" sounds like "twerp", which means something like "insignificant pest."

We currently have the /twirp prefix to avoid colliding with gRPC's routes, which live at /package.service/method, and to reserve a space for future APIs (like a reflection server).

Slashes-based routes (instead of dot-based) would be preferable because lots of existing technology expects slash-based routing. Load balancers and proxies use slashes for namespaces. If Twirp used a slash after the package name, users could leverage this much more easily. This is often possible with dots, just annoying, as you have to write a (possibly pretty complex) regex.

I think we can fix both of these in one go. This would be a breaking change at the protocol level, so it's something to take very seriously, but I think we can do it without much pain. It would just take a version bump to v6.

Proposed Implementation:

I propose we use a new routing scheme: /<package>/<service>/<method>.

This does not collide with gRPC's routes, which is good. I think it gives us flexibility to put future routes in by putting them under a Twirp package. We could provide a reflection service like this one, which lists available services:

syntax = "proto3";

package twirp.meta

import "github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor.proto"

service Reflector {
  rpc ListServices(ListServiceRequest) returns (ListServiceResponse);
}

message ListServiceRequest {}
message ListServiceResponse {
  repeated protobuf.ServiceDescriptorProto services = 1;
}

It would live at /twirp.meta/Reflector, which seems good to me.

Regarding compatibility: generated clients would always use the new routing scheme. Generated servers would be able to use both, in case your users have old generated clients still around. We would accomplish this by exporting two path prefixes:

const (
  HaberdasherPathPrefix = "/notreal.example/Haberdasher/"
  HaberdasherLegacyPathPrefix = "/twirp/notreal.example.Haberdasher/"
)

Users can mount both on a mux if they want to support old clients. If not, they can just use the new one.

If we do this, there are other consequences.

• Documentation would need to be updated everywhere.
• Server implementations in other languages would need to be updated ASAP, as new clients won't work with old servers.
• If people have log parsing or metrics gathering code that operates on routes, they'd need to change them.

The pain of those other consequences is a good reason that we should try to get this out promptly, if we do it.

I think this is a great addition for RPC for Go (as well as other languages!). The main thing that I expect will limit adoption is the lack of streaming support. There are mechanisms to do streaming on http/1.x (a great example is the grpc-gateway stuff) where they effectively just use chunked encoding with blobs to do the streaming.

Not sure how to PR wikis, but there is a typo in the usage of the client code. I believe the last section should have this error handling code since the current code doesn't compile.

if err != nil {
    twerr, ok := err.(twirp.Error)
    if ok {
        switch twerr.Code() {
        case twirp.InvalidArgument:
            fmt.Println("Oh no "+twerr.Msg())
        default:
            fmt.Println(twerr.Error())
        }
    }
    return
}

Hello,

On the Errors wiki package there's a typo:

• https://github.comm/twitchtv/twirp/blob/master/errors.go
  should be
• https://github.com/twitchtv/twirp/blob/master/errors.go (.com and not .comm)
  which means the link is dead.

I couldn't find a way to submit a pull request for the wiki repository so I'm opening an issue. Let me know if there's a better way.

Cheers!

I may be thinking about using twirp.Error the wrong way and if so please ignore me ;).

I noticed that twirp.Error() always returns a string in the form twirp error <Type>: <Msg> per the docs: https://godoc.org/github.com/twitchtv/twirp#Error

Example from a cli using a generated JSON client:

2018/01/28 15:24:57 twirp error already_exists: entity must be unique

It would be nice if I could somehow configure the format of this error message to not include twirp error. This seems somewhat related to #55 in that the name twirp may throw some people off or you may want to 'hide' implementation details (twirp).

I realize that we can always check for twirp.Errors and then provide our own error message like what is shown in your docs:

if err != nil {
    if twerr := err.(twirp.Error) {
        switch twerr.Code() {
        case twirp.InvalidArgument:
            log.Error("invalid argument "+twirp.Meta("argument"))
        default:
            log.Error(twerr.Error())
        }
    }
}

But it may be helpful to not have to do this every time if you just want to print out the error message without twirp error <Type>: being prepended.

First of all, thank you for this package! It is awesome and super simple to use. Great job!

I've got my message definition

message Points {
  int32 team_a = 1;
  int32 team_b = 2;
}

which is translated into a Go type.

type Points struct {
	TeamA int32 `protobuf:"varint,1,opt,name=team_a,json=teamA" json:"team_a,omitempty"`
	TeamB int32 `protobuf:"varint,2,opt,name=team_b,json=teamB" json:"team_b,omitempty"`
}

I'm using JSON to communicate between browser and twirp backend. Now I've got a match where team_a = 2 and team_b = 0. In the browser the value for team_b is missing since 0 is the zero value for int. Those are left out due to the omitempty flag.

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

That's a bit annoying :)

Any ideas how to make it work? How do you handle those cases at Twitch?

The protocol as it stands only allows for either a response or an error to be returned. There are plenty of situations where something may want to return a valid value as well - think the io.Writer interface.

One way with HTTP to do this is to define response headers, something like 'rpc-error-code' and -'rpc-error-message', that carry this information in the headers. This became useful to use. Extending this paradigm to allow complex error returns is a topic for debate.

In terms of this protocol, you'd have to do some content negotation for backwards compatibility - think sending 'rpc-accepts-both-response-error' to denote you can handle this new form. We ran into this exact problem https://github.com/yarpc/yarpc-go/blob/dev/transport/http/constants.go#L79

Thoughts?

What I did

I created a service method that takes no inputs. It has a request message type, but that type has no fields.

service Geo {                               
  rpc Get(GetRequest) returns (GetResponse);
}                                           
                                            
message GetRequest {                        
}                                           
                                            
message GetResponse {                       
  string country = 1;                       
}                                           

I made a request to the server with content-type application/json and an empty body.

curl \
  --request POST \
  --header "Content-Type: application/json" \
  http://localhost:8080/twirp/app.Geo/Get

What happened

The server responded with an error.

{"code":"internal","msg":"failed to parse request json: EOF","meta":{"cause":"*app.wrappedError"}}

What I thought would happen

I thought the server would response with the result of the method.

{"country":"US"}

What I needed to do to make it work

I added an empty json object as the body of the request.

curl \
  --request POST \
  --header "Content-Type: application/json" \
  --data '{}' \
  http://localhost:8080/twirp/app.Geo/Get

Proposal

Support empty request body on requests with content-type application/json.

Hi all,

The generated urlBase function doesn't seem to be working as intended, or the New{Service}ProtobufClient documentation needs to be updated.

Here's a small program highlighting the problem:

package main

import (
	"fmt"
	"net/url"
)

// this is a copy of urlBase as found in .twirp.go files
func urlBase(addr string) string {
	// If the addr specifies a scheme, use it. If not, default to
	// http. If url.Parse fails on it, return it unchanged.
	url, err := url.Parse(addr)
	if err != nil {
		return addr
	}
	if url.Scheme == "" {
		url.Scheme = "http"
	}
	return url.String()
}

func main() {
	const addr = "127.0.0.1:1234"
	fmt.Println(urlBase(addr))
	fmt.Println(url.Parse(addr))
}

127.0.0.1:1234
<nil> parse 127.0.0.1:1234: first path segment in URL cannot contain colon

The problem is when the HTTP request is constructed, it needs a scheme (either http:// or https://) or it fails with the same error, because it mistakens the IP:port pair for a path in the URL.

Prefixing the address with "http://" when instantiating the client worked fine.

This was confusing and hard to track down tho, I suggest we do one of the following:

• Document the New{Service}ProtobufClient function to mention that the address must have a scheme (which means the urlBase function could go away)
• Modify the urlBase function to not rely on url.Parse, maybe just look for a :// sequence instead.

Let me know what you think would be best, or if you have other recommendations, I can take care of submitting a PR.

Twirp's specification is simple, but it's described in a lot of different places. We would benefit from having one spec that's authoritative, written in one place.

@wora, who worked on proto3 and grpc, has kindly offered to assist in drafting a spec for Twirp.

Hopefully, this will make it easier for people to write servers.

Hi!

A minor suggestion: make it so that the example (https://github.com/twitchtv/twirp/wiki/Usage-Example:-Haberdasher) is in a separate repo, that we can git clone, cd into and follow simple instructions to have a running server and client.

Its just an idea, but I've noticed this on other projects and makes everything super easy for beginners (even though Twirp isn't exactly for beginners, coming from Twitch a lot of people might want to play with it).

Cheers!

I am road testing twirp with amazon api gateway and lambda, this service requires the Accept header as this triggers binary encoding of the response for application/protobuf.

I am happy to do a PR for this.

I really like this library and think it would benefit from being able to support both standard RPC as well as GRPC as well. Looking through the code it seems to only generate standard HTTP RPC bindings. Is there interest in also supporting GRPC as well in the same implementation? For certain use cases (at least unary) having support for both is important to us. Seems like it would be rather easy to add support for GRPC as well and you get the best of both worlds without the silly grpc gateway mess.

Hi, can you suggest what load balancing/service discovery strategies do you use in twitch?
As I see the current implementation of twirp allows only to use server-side discovery using some reverse-proxy like ELB, nginx, hproxy, etc. But it will be great to have the possibility to use client-side discovery as well.
It becomes easier to implement it with #39 because I can replace URL with the discovered host in the request. So imagine I can write some custom implementation of HTTPClient interface or http.RoundTripper like:

// ClientWithDiscovery is an implementation of HTTPClient interface that automatically discovers services' hosts using consul/etcd/...
type ClientWithDiscovery struct {
   service string
   client    HTTPClient
}

func (c *ClientWithDiscovery) Do(r *http.Request) (*http.Response, error) {
    r.URL.Host = c.pickHostForService()
    return c.client.Do(r)       
}

The big problem of such implementation is that service is a property of the client and I have to create separate client for each service I want to use. But it can be easily solved if in request's context there will be information about service. Client generated code can contain lines like:

func (c *haberdasherJSONClient) MakeHat(ctx context.Context, in *Size) (*Hat, error) {
        ctx = ctxsetters.WithServiceName(ctx, "Haberdasher") // <-- new line
	url := c.urlBase + HaberdasherPathPrefix + "MakeHat"
	out := new(Hat)
	err := doJSONRequest(ctx, c.client, url, in, out)
	return out, err
}

So then I can call twirp.ServiceName(ctx) in my new client to fetch service name and make discovery.

What do you think?

Each generated twirp service for go gets it's own definition of HTTPClient and TwirpServer see:

This raises issues with storing multiple "different" TwirpServer instances for e.g. within an array. I would suggest moving this parts into twirp itself which removes repetition and unifies the handling.

If welcome I'm willing to put together a PR.

Noting that I don't see any Apache 2.0 license headers on the source files.

I would like to discuss the design of the new client functions, to propose a new one with functional options

Currently we need this to create a client:

client := haberdasher.NewHaberdasherProtobufClient("http://localhost:8000", &http.Client{})
client := haberdasher.NewHaberdasherJSONClient("http://localhost:8000", &http.Client{})

This could be simplified to sane defaults & still allow for customizations of everything with a single function:

// Defaults to protobuf & http.DefaultClient.
client := haberdasher.NewHaberdasherClient("http://localhost:8000")

// Customize the client
client := haberdasher.NewHaberdasherClient("http://localhost:8000", twirp.WithClient(&http.Client{}))

// Customize the format.
client := haberdasher.NewHaberdasherClient("http://localhost:8000", twirp.WithJSON())

// Customize the format & client.
client := haberdasher.NewHaberdasherClient("http://localhost:8000", twirp.WithClient(&http.Client{}), twirp.WithJSON())

This would also give freedom to add new options if a new requirement is found.

What do you think?

It would be great to remove dependency to http.Client type. Instead of this interface can be used:

// HTTPClient sends http.Requests and returns http.Responses or errors in case of failure.
type HTTPClient interface {
	Do(req *http.Request) (*http.Response, error)
}

It allows creating decorators for standard http.Client with some additional functionality like metrics, backoff retries, etc. http.Client implements this interface and such code is backward compatible with the current implementation. We use such approach and it's great.

What do you think? If you like this idea I can make PR.

Hi there!

Thanks for bringing this framework to us 👍

I've been playing around with it for a bit today, and it looks like the generation of the imports has an issue. Either that or I'm just missing something so I hope you can help me out!

I'm generating the .twirp.go file using this command line:

protoc -I . --twirp_out=./out --go_out=./out service_def.proto dep1.proto dep2.proto...

In the generated twirp service file I have the following import:

import mypackagename "."

But this cannot work when I want to use the generated source from an external package, and of course I get the following error: local import "." in non-local package on my external package (implemetation) when I import a message type in my source code.

The upper part of my service is as follows:

syntax = "proto3";

package some.thing;
option go_package = "mypackagename";

Running make from master/v5.0.0 (db96cdf) without any changes modifies 9 python test files. I expected no changes to checked-in files.

Each includes additional code to register a service descriptor, similar to:

_SVC = _descriptor.ServiceDescriptor(
  name='Svc',
  full_name='twirp.internal.twirptest.proto.Svc',
  file=DESCRIPTOR,
  index=0,
  options=None,
  serialized_start=54,
  serialized_end=141,
  methods=[
  _descriptor.MethodDescriptor(
    name='Send',
    full_name='twirp.internal.twirptest.proto.Svc.Send',
    index=0,
    containing_service=None,
    input_type=_MSG,
    output_type=_MSG,
    options=None,
  ),
])
_sym_db.RegisterServiceDescriptor(_SVC)

DESCRIPTOR.services_by_name['Svc'] = _SVC

Should these files be updated with these changes?

#3 lays out some ideas for how Twirp could support streaming RPCs on the wire. Equally important is how we support them in generated code.

This issue is for designing the generated Go code. When we've landed on a proposal that seems pretty good, I'll edit this top text with what we've got, and then we can look at implementing it.

Let's use this service to discuss what things would look like:

syntax = "proto3";

service Streamer {
  rpc Transaction(Req) returns (Resp);
  rpc Upload(stream Req) returns (Resp);
  rpc Download(Req) returns (stream Resp);
  rpc Bidirectional(stream Req) returns (stream Resp);
}

message Req {}
message Resp {}

CONTRIBUTING.md says:

Twirp uses github pull requests. Fork a branch from develop, ...

but no develop branch exists. Are these docs stale? Or has develop just not yet been pushed to the public repo?

I was trying twirp today to add some calls to an existing service which uses the gorilla mux (https://github.com/gorilla/mux). But the handler from twirp wasn't working with the mux from gorilla. Is this intentional or something we could support in the future?

The mux from gorilla implements the http.Handler interface and also the Handle method for registring a http.Handler with a specific path.

I have an external service called "Metadata", which is a twirp server.

Here's a small test app I wrote to try to send it an HTTP header, basically following the example in the wiki verbatim.

package main

import (
	"context"
	"log"
	"net/http"
	"os"

	pb "git.example.io/rune/rune/metadata/rpc"
	"github.com/twitchtv/twirp"
)

var (
	twirpContext = context.Background()
)

func init() {
	header := make(http.Header)
	log.Println(header)
	header.Set("Example-Authorization", "testing")
	var err error
	twirpContext, err = twirp.WithHTTPRequestHeaders(twirpContext, header)
	if err != nil {
		log.Panicln("Error attaching headers to twirp context: ", err.Error())
	}
}

func main() {
	log.Println(twirpContext)
	exampleClient := pb.NewMetadataProtobufClient(os.Getenv("RUNE_METADATA_ADDRESS"), &http.Client{})
	exampleClient.Get(twirpContext, &pb.GetMetadataRequest{})
}

Log output:

2018/03/27 13:17:23 map[]
2018/03/27 13:17:23 context.Background.WithValue(5, http.Header{"Example-Authorization":[]string{"testing"}})

Here is the request being logged when Go / twirp is used:

...metadata.Metadata/Get] Headers: map[Content-Type:[application/protobuf] Twirp-Version:[v5.2.0] Accept-Encoding:[gzip] X-Envoy-Internal:[true] X-Envoy-Expected-Rq-Timeout-Ms:[5000] User-Agent:[Go-http-client/1.1] Content-Length:[0] X-Forwarded-Proto:[https] X-Request-Id:[eb43abc0-bbe9-4d7e-9f37-f709e82949e1] X-Ambassador-Calltype:[extauth-request] X-Forwarded-For:[10.24.5.119]]

You'll notice Twirp-Version is there, but not Example-Auth.

But when I use curl:

curl -L -v -X POST -H "content-type: application/json" -d '{"test":"req"}' -H "Example-Authorization: testing" https://api.example.io/metadata/v1/twirp/example.metadata.Metadata/Get

...metadata.Metadata/Get] Headers: map[Content-Length:[0] X-Forwarded-Proto:[https] X-Ambassador-Calltype:[extauth-request] X-Forwarded-For:[10.24.5.119] User-Agent:[curl/7.59.0] Accept:[*/*] Content-Type:[application/json] Example-Authorization:[testing] X-Request-Id:[2177d537-6079-42fb-9dd9-15c0e96feb25] X-Envoy-Internal:[true] X-Envoy-Expected-Rq-Timeout-Ms:[5000]]

To me, the fact that it works with curl but not Twirp says that the problem is that twirp is not including the headers in the request.

Edit: This happens regardless of what the content-type is set to. (pb.NewMetadataProtobufClient vs pb.NewMetadataJSONClient)

Hi,

the server code in your example is faulty:

func (s *Server) MakeHat(ctx context.Context, size *pb.Size) (hat *pb.Hat, error) {

should be:

func (s *Server) MakeHat(ctx context.Context, size *pb.Size) (hat *pb.Hat, err error) {

you need to add the err, otherwise the following compiler error pops up:
syntax error: mixed named and unnamed function parameters

Cheers

Hello,

I was really excited to read about twirp and want to undertake a project with it. Unfortunately, twirp seems to make some pretty opinionated decisions about my API's pathing. I've noticed that the generator prepends the literal "twirp" to the url, and seems to require the services internal name be exposed. I'm not familiar with writing proto generators but I assume it is possible to provide some sort of generator option to disable or customize this functionality.

For the hard coded /twirp, I'm wondering if this is a twitch specific thing or if you're open to removing it. Considering I can use my own mux, I'm capable of prepending paths already.

For the service name, it seems a bit less straight forward. It is possible to expose the handlers so they can be plugged directly into the mux. I guess the apprehension is keeping twirp from becoming a complex mux in and of itself, but I think it can be made possible without convoluting the code base.

There are several code issues on the https://github.com/twitchtv/twirp/wiki page. While nothing is advertising those snippets as 100% perfect, it'd be nice to have the egregious errors fixed. Notably:

Server snippet:

func (s *HelloWorldServer) Hello(ctx context.Context, req *pb.HelloReq) (hat *pb.HelloResp, error) {

the return values mix named and unnamed returns, which is unallowed. Consider removing the "hat" name.

Client snippet

The client created is targeting port 8000, where the server was put on port 8080.

The client is also doing reversed logic err check:

if err != nil {
    fmt.Println(resp.Text) // prints "Hello World"
}

which should only print anything if an error was returned, which is almost assuredly the wrong thing to be doing here.

In addition, I'd consider changing resp, err = client.Hello( to resp, err := client.Hello(, as there's nothing in this snippet creating resp or err, so it would seem cleaner this way

Would using circle-ci or travis for the repository help? It would allow people to submit pull requests and see the build result of them in the github UI.

Mentions that these deps are only needed for testing. This is however, untrue. They are also needed to build the generators.

Currently twirp ships with statsd server hook. I would like to use Prometheus for my metrics.

Are contributions for other server hooks welcome in twirp repository or should they be separate projects?

I adapted the current statsd hook for Prometheus: https://github.com/joneskoo/twirp-serverhook-prometheus . I had to copy some internal stuff for tests, which either suggests to me they should be exported or that this hook should go to twirp repository. Turns out the prometheus client is much simpler than statsd due to labels being easier to work with.

Depending on response I can send a PR.

This obviously overlaps with #55, but the discussion there focused on removal of the /twirp prefix. I'd like a new issue to concentrate discussion on just the . to / switch.

The switch from a . to delimit package and service towards / as delimiter is really consequential and needs its own clearer discussion. Let's work out whether it's a good idea.

Arguments in favor

Path-based routing in other infrastructure

Lots of intermediary proxies, load balancers, caches, and so on will naturally split a URL on / characters. Using the / character will make it easier to configure those tools. For example, a load balancer could more simply route requests to the right backend if it knew that the second path component is the service name.

Arguments against

Long-term cruft in generated APIs

We are removing the /twirp prefix, and instead permitting user-defined prefixes.

Migrating a server from v5 to v6 is easy if this is the only change. To support v5 clients, you do this:

mux := http.NewServeMux()
mux.Handle(HaberdasherPathPrefix, habderdasher)
mux.Handle("/twirp" + HaberdasherPathPrefix, http.StripPrefix("/twirp", haberdasher))

If we change the . to / too, though, we need to export an additional path prefix const. We'll need these:

const (
  HaberdasherPathPrefix = "/twirp.example/Haberdasher/"
  HaberdasherV5PathPrefix = "/twirp/twirp.example.Haberdasher/"
)

And the user will need to use the right one in their mux:

mux := http.NewServeMux()
mux.Handle(HaberdasherPathPrefix, habderdasher)
mux.Handle(HaberdasherV5PathPrefix, haberdasher)

Generated code would need to route these v5 requests correctly in addition to the new v6 requests. We'd need backwards compatibility shims in generated code, probably forever, and we would need to keep the extra exported const around forever.

Route ambiguity when package is unset

Proto files need not set package. What do we do in those cases? @rhysh outlined the way this could go wrong. For illustration, suppose you had these two services:

package Foo

service Bar {
  rpc Handle(Req) returns (Resp);
}

and

// package is never specified

service Foo {
  rpc Bar(Req) returns (Resp);
}

What are the routes? If we leave ., the routes are:

  /Foo.Bar/Handle -> Bar service's Handle method
  /Foo/Bar        -> Foo service's Bar method

If we change them to /, though, they are:

  /Foo/Bar/Handle -> Bar service's Handle method
  /Foo/Bar        -> Foo service's Bar method

With ., it's obvious how you route requests. Requests prefixed with /Foo.Bar/ go to the Bar service; requests prefixed with /Foo/ go to the Foo service.

But with /, it's not clear. The two services collide.

I've created a protoc twirp client generator that outputs fairly simple typescript. I've already tested it out on a few personal projects and I've been happy with the amount of code I've been able to delete on the client and server.

I'm looking for feedback from the twirp team to see if it should be added to the list of codegen plugins in the twirp docs.

@spenczar this is the project I referenced in issue #84 , if you could take a few minutes to comment I'd appreciate the feedback to see if it's something that is useful for the general twirp community.

CONTRIBUTING.md states:
Generally you want to make changes and run make, which will install all dependencies we know about...
but I experienced build failures (with no changes). However, running dep init to vendor dependencies first led to a successful run of make, suggesting something in my GOPATH was incompatible/outdated and not updated by make.

• Are there additional dependencies to install/update via make?
• Alternatively, could a dep manifest and/or vendor directory be used to stabilize dependencies? (or would that clash with the rest of make, which is built around GOPATH and go install?)

Trying to use hooks for authentication and authorization does not seem possible as stated in the documentation, especially if auth token are present in the header of each request.

I'm using JWT with the token in the authorization header.

This is a copy pasta from #15:

When attempting to import descriptor.proto I run into a Go import issue on the compiled Go.
"google/protobuf/descriptor.proto";

Reproducible:

syntax = "proto3";
import "google/protobuf/descriptor.proto";
package twitch.twirp.example;
option go_package = "example";

extend google.protobuf.MessageOptions {
    string some_key = 1000;
}

message Hat {
  option (some_key) = "a hat!"
  int32 size = 1;
  string color = 2;
  string name = 3;
}

message Size {
  int32 inches = 1;
}

service Haberdasher {
  rpc MakeHat(Size) returns (Hat);
}

Yields the following Go imports:

import proto "github.com/golang/protobuf/proto"
import fmt "fmt"
import math "math"
import google_protobuf "google/protobuf"

Which obviously yields the error:
cannot find package "google/protobuf"

I verified that importing something like google/protobuf/timestamp.proto does work correctly, it's just the descriptor.proto that doesn't.

Referencing this part of the generated code:

Right now the jsonpb.Marshaler is configured to use the original name of the proto Message, instead of doing the default camel casing.

Is the design of twirp that the proto and json serialization should always be equivalent? It would be less work to do idiomatic javascript twirp clients if twirp allowed jsonpb to do its default behavior of lowerCamelCasing Message fields:

https://developers.google.com/protocol-buffers/docs/proto3#json

If this isn't an option for the v5 release, can it be debated for the current v6 work since it would be a breaking change?

The generated code for server expects Header with Content-Type of either application/json or application/protobuf.

Servers deal with the Accept header to determine the accept response type and servers generate the Content-Type header based on this content of the media returned. https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

I can hack the client to send a Content-Type and it works but this doesn't seem to make sense to me.

(Happy to provide a PR that would check for Accept and fallback to Content-Type for backwards compat if you're in agreement)

We want to support go1.7+, but our tests use t.Helper, which was added in go1.9.

We should be able to run tests in go1.7. If that means dropping t.Helper or making a shim through build tags, so be it.

i’m interested in ruby and java support (maybe php too). any advice along these lines?

There has been a really incredible burst of community-written generators. I want to highlight them and send people towards them.

Let's add a bit to the README, close to the top, that lists the generators out there for different languages.

I think the standard here should be that we list generators that make valid a valid client from a protobuf file, will be actively maintained, and which are in non-go environments. It can be a pretty wide list right now. In the future, we may get more picky, but for now, let's just list what's out there.

The wiki has an example of how to curl using the json format here:
https://github.com/twitchtv/twirp/wiki/HTTP-Routing-and-Serialization#making-requests-on-the-command-line-with-curl

It would be useful I think if this section also contained an example of how to curl using protobuf, which is possible if the user has protoc installed, which is likely since it's required for building the autogenerated code.

e.g.

echo "inches:10" \
	| protoc --proto_path=$GOPATH/src --encode twirp.example.haberdasher.Size ./rpc/haberdasher/service.proto \
	| curl -s --request POST --header "Content-Type: application/protobuf" --data-binary @- http://localhost:8080/twirp/twirp.example.haberdasher.Haberdasher/MakeHat \
	| protoc --proto_path=$GOPATH/src --decode twirp.example.haberdasher.Hat ./rpc/haberdasher/service.proto

I'd open a PR to add it to docs, but I don't see a way of doing that since the docs are in the github wiki and not in the repo.

Some of my Services have empty request bodies.

For example:

message BarRequest {
}

message BarResponse {
   string hey = 1;
}

service Foo {
	// ReleaseList will return a list of releases
	rpc Bar(BarRequest) returns (BarResponse);
}

In this case, the client should be able to send a POST with an empty body (which is accept based on HTTP spec). However, this generates an error such as:

{"code":"internal","msg":"failed to parse request json: EOF","meta":{"cause":"*v1.wrappedError"}}

I believe the server should check the Content-Length header before trying to unmarshal the request (and let the implementation handle if required).

(Happy to provide a PR that would fix this if you're OK with this)

Anyone tried to run Twirp on Lambda yet?

I'm not a fan of API Gateway (APIG) and all the complexity, but I love lambda. I think it would be sweet to do the following:

1. Configure APIG to handle ANY method on the /{proxy+} resource will reverse proxy everything to one Lambda function (lambda function is a go binary using Twirp).
2. HTTP 1.1 application/json request comes into APIG, it gets routed to Lambda
3. Thin go handler code transforms the events. APIGatewayProxyRequest (github.com/aws/aws-lambda-go/events) to the HTTP request object that Twitch needs.
4. Twitch router works as normal
5. Thin layer that takes the Twirp response and transforms it to a return events.APIGatewayProxyResponse

Questions (I'm a go noob, never used RPC or protobuf but they make sense to me):

1. Am I missing anything big here that would prevent me from doing this?
2. Can anyone point me to the Twirp code that would get me started on 3 and 5 above?

I've been playing around with trying to get Twirp running on App Engine Standard. Technically it works, but to call any methods in the appengine package you need an appengine.Context which can only be created using the http.Request object.

So, any thoughts on if its possible to get the Request object?

I've started work on a generating swagger files for twirp services (almost 100% based on https://github.com/grpc-ecosystem/grpc-gateway/tree/master/protoc-gen-swagger)

The clients would be JSON only (I think?), but still. Pretty useful to get working clients for java, .net, etc.. pretty quickly using https://github.com/swagger-api/swagger-codegen#overview

Would you prefer it as an external plugin (will you have a community repo or something?) or as part of the main package?

Example output from your sample proto..

{
  "swagger": "2.0",
  "info": {
    "title": "test.proto",
    "version": "version not set"
  },
  "schemes": [
    "http",
    "https"
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/twirp/twitch.twirp.example.Haberdasher/MakeHat": {
      "post": {
        "summary": "MakeHat produces a hat of mysterious, randomly-selected color!",
        "operationId": "MakeHat",
        "responses": {
          "200": {
            "description": "",
            "schema": {
              "$ref": "#/definitions/exampleHat"
            }
          }
        },
        "parameters": [
          {
            "name": "body",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/exampleSize"
            }
          }
        ],
        "tags": [
          "Haberdasher"
        ]
      }
    }
  },
  "definitions": {
    "exampleHat": {
      "type": "object",
      "properties": {
        "size": {
          "type": "integer",
          "format": "int32",
          "description": "The size of a hat should always be in inches."
        },
        "color": {
          "type": "string",
          "description": "The color of a hat will never be 'invisible', but other than\nthat, anything is fair game."
        },
        "name": {
          "type": "string",
          "description": "The name of a hat is it's type. Like, 'bowler', or something."
        }
      },
      "description": "A Hat is a piece of headwear made by a Haberdasher."
    },
    "exampleSize": {
      "type": "object",
      "properties": {
        "inches": {
          "type": "integer",
          "format": "int32"
        }
      },
      "description": "Size is passed when requesting a new hat to be made. It's always\nmeasured in inches."
    }
  }
}

Example: https://github.com/yarpc/yarpc-go/blob/dev/internal/protoplugin/runner.go#L50

Will be needed when request/response types are outside of the package of the service, or if generating the Twirp handlers to a different golang package.

Nice work by the way!

I looked at several places before scrolling down and finally finding your guide :)