Create PROTOCOL.md #71
Conversation
Draft the wire protocol spec for Twirp based on latest discussions. It reflects the existing Twirp wire spec, plus best practice from proto3, gRPC, HTTP, and JSON. The spec should be usable for developers to build clients and servers using Twirp wire protocol without much dependency.
|
|
|
Charles Proxy seems to support either |
|
Google didn't complete its IETF application, https://tools.ietf.org/html/draft-rfernando-protocol-buffers-00. So technically it still is |
PROTOCOL.md
Outdated
|
|
||
| The Twirp wire protocol supports both binary and JSON encodings of | ||
| proto messages, and works with any HTTP client and any HTTP version. | ||
| However, certain capabilities may be limited by the actual HTTP |
There was a problem hiding this comment.
I don't think it's worth including this caveat until we can actually say this precisely.
PROTOCOL.md
Outdated
|
|
||
| ### URLs | ||
|
|
||
| **URL ::= Base-URL "/" [ Package "." ] Interface "/" Method** |
There was a problem hiding this comment.
Please change Interface to Service.
Also, the current routing is URL ::= Base-URL "/twirp/" [ Package "." ] Service "/" Method, please update this to match the /twirp prefix.
PROTOCOL.md
Outdated
|
|
||
| **URL ::= Base-URL "/" [ Package "." ] Interface "/" Method** | ||
|
|
||
| The Twirp wire protocol uses HTTP URLs to directly specify the RPC |
PROTOCOL.md
Outdated
| **URL ::= Base-URL "/" [ Package "." ] Interface "/" Method** | ||
|
|
||
| The Twirp wire protocol uses HTTP URLs to directly specify the RPC | ||
| endpoints on the server for sending the requests. such direct mapping |
PROTOCOL.md
Outdated
|
|
||
| * **Base-URL** is the virtual location of a Twirp API server, which is | ||
| typically published via API documentation or service discovery. For | ||
| example, "https://example.com/apis". |
There was a problem hiding this comment.
This is a bad example right now, as the base URL in v5 cannot safely include a path component.
I think we should be more specific here. The Base URL should contain only the "scheme" and "authority" (commonly known as "host and port") portions of a RFC 3986 URI.
We will likely change this to allow a path component in v6, as discussed in #55.
PROTOCOL.md
Outdated
|
|
||
| ### Network errors | ||
|
|
||
| If a client fails to reach the server due to network errors, the |
There was a problem hiding this comment.
I don't think we should specify this. I don't think client libraries should be exposing HTTP status codes, just Twirp error codes.
PROTOCOL.md
Outdated
| If an error occurs when the server processes a request, the server | ||
| must return an error payload as the response message, and correctly | ||
| set the HTTP status code. Please see | ||
| [`google.rpc.Code`](https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto) |
There was a problem hiding this comment.
We don't strictly follow google.rpc.Code. We use our own error codes, heavily based on those but with some modifications: the codes are a string on the wire for human readability, and we add the bad_route error code. See https://github.com/twitchtv/twirp/blob/master/errors.go#L138-L275.
PROTOCOL.md
Outdated
| ## Errors | ||
|
|
||
| If an error occurs when the server processes a request, the server | ||
| must return an error payload as the response message, and correctly |
There was a problem hiding this comment.
I think we should describe Twirp's error payload in detail here.
Twirp error responses are always JSON-encoded (regardless of the request's Content-Type), with a corresponding Content-Type: application/json header. This ensures that they are human-readable in any setting.
They are a JSON object with three keys:
- code: One of the Twirp error codes as a string.
- msg: A human-readable message describing the error as a string.
- meta: An object with string keys and values holding arbitrary additional metadata describing the error.
For example:
{
"code": "permission_denied",
"msg": "thou shall not pass",
"meta": {
"target": "Balrog"
}
}
PROTOCOL.md
Outdated
|
|
||
| ## Conventions | ||
|
|
||
| The requirement level keywords "MUST", "MUST NOT", "REQUIRED", |
There was a problem hiding this comment.
None of these keywords are used in this document. Let's drop this paragraph.
PROTOCOL.md
Outdated
| and "OPTIONAL" used in this document are to be interpreted as | ||
| described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt). | ||
|
|
||
| The grammar rules used in this document are using [ABNF |
There was a problem hiding this comment.
We only use ABNF in one place where it's useful, which is in describing the URL. Let's delete this paragraph and just say something like In [ABNF syntax](https://tools.ietf.org/html/rfc5234), Twirp's URLs follow this format: ...
Then, we can get rid of this Conventions section altogether, which is good.
Addressing review comments.
|
Should be all fixed. |
PROTOCOL.md
Outdated
| **Proto Request** | ||
|
|
||
| ``` | ||
| POST /twirp.Echo/Hello HTTP/1.1 |
There was a problem hiding this comment.
The URL here should be /twirp/twirp.Echo/Hello. Same for JSON.
The stuttering of 'twirp' here is not ideal. Could you change the package to something like package example.echoer?
Change proto package name to example.echoer.
|
Done. |
Use google/protobuf.
Draft the wire protocol spec for Twirp based on latest discussions. It reflects the existing Twirp wire spec, plus best practice from proto3, gRPC, HTTP, and JSON. The spec should be usable for developers to build clients and servers using Twirp wire protocol without much dependency.
Issue #, if available:
#71
Description of changes:
Write up the Twirp wire protocol.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.