{"action":"created","issue":{"url":"https://api.github.com/repos/square/okhttp/issues/4267","repository_url":"https://api.github.com/repos/square/okhttp","labels_url":"https://api.github.com/repos/square/okhttp/issues/4267/labels{/name}","comments_url":"https://api.github.com/repos/square/okhttp/issues/4267/comments","events_url":"https://api.github.com/repos/square/okhttp/issues/4267/events","html_url":"https://github.com/square/okhttp/issues/4267","id":359892186,"node_id":"MDU6SXNzdWUzNTk4OTIxODY=","number":4267,"title":"Duplex Calls","user":{"login":"swankjesse","id":133019,"node_id":"MDQ6VXNlcjEzMzAxOQ==","avatar_url":"https://avatars3.githubusercontent.com/u/133019?v=4","gravatar_id":"","url":"https://api.github.com/users/swankjesse","html_url":"https://github.com/swankjesse","followers_url":"https://api.github.com/users/swankjesse/followers","following_url":"https://api.github.com/users/swankjesse/following{/other_user}","gists_url":"https://api.github.com/users/swankjesse/gists{/gist_id}","starred_url":"https://api.github.com/users/swankjesse/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swankjesse/subscriptions","organizations_url":"https://api.github.com/users/swankjesse/orgs","repos_url":"https://api.github.com/users/swankjesse/repos","events_url":"https://api.github.com/users/swankjesse/events{/privacy}","received_events_url":"https://api.github.com/users/swankjesse/received_events","type":"User","site_admin":false},"labels":[{"id":9998867,"node_id":"MDU6TGFiZWw5OTk4ODY3","url":"https://api.github.com/repos/square/okhttp/labels/enhancement","name":"enhancement","color":"84b6eb","default":true}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/square/okhttp/milestones/25","html_url":"https://github.com/square/okhttp/milestone/25","labels_url":"https://api.github.com/repos/square/okhttp/milestones/25/labels","id":2743829,"node_id":"MDk6TWlsZXN0b25lMjc0MzgyOQ==","number":25,"title":"Backlog","description":"Triaged issues that haven’t yet been assigned a specific release.","creator":{"login":"swankjesse","id":133019,"node_id":"MDQ6VXNlcjEzMzAxOQ==","avatar_url":"https://avatars3.githubusercontent.com/u/133019?v=4","gravatar_id":"","url":"https://api.github.com/users/swankjesse","html_url":"https://github.com/swankjesse","followers_url":"https://api.github.com/users/swankjesse/followers","following_url":"https://api.github.com/users/swankjesse/following{/other_user}","gists_url":"https://api.github.com/users/swankjesse/gists{/gist_id}","starred_url":"https://api.github.com/users/swankjesse/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swankjesse/subscriptions","organizations_url":"https://api.github.com/users/swankjesse/orgs","repos_url":"https://api.github.com/users/swankjesse/repos","events_url":"https://api.github.com/users/swankjesse/events{/privacy}","received_events_url":"https://api.github.com/users/swankjesse/received_events","type":"User","site_admin":false},"open_issues":88,"closed_issues":2,"state":"open","created_at":"2017-09-04T22:02:25Z","updated_at":"2018-12-05T15:41:09Z","due_on":null,"closed_at":null},"comments":6,"created_at":"2018-09-13T13:18:48Z","updated_at":"2018-12-10T19:50:01Z","closed_at":null,"author_association":"MEMBER","body":"# Motivation\r\n\r\nI’d like to build make it possible for gRPC clients to be built with OkHttp’s public API. The gRPC project already has an [OkHttp backend](https://github.com/grpc/grpc-java/tree/master/okhttp/src/main/java/io/grpc/okhttp), but it needs to rely on internal features.\r\n\r\nThere’s two things gRPC needs that OkHttp doesn’t offer:\r\n\r\n * Sending data on the request stream _after_ having received the response status and headers. This isn’t like HTTP because it isn’t request/response. It’s a bit more like WebSockets. It’s possible to do a two-way chat session with a single gRPC call.\r\n * Sending and receiving trailers. This is how gRPC cleanly signals a clean termination of a stream.\r\n\r\nI’d like to build APIs on OkHttp to support these. At the moment gRPC would be the only use case for these APIs, but we wouldn’t label them as “gRPC”. The name I like best is “duplex” but I’d love to find something better.\r\n\r\n# API\r\n\r\nHere’s a sketch of how duplex calls would work.\r\n\r\n### Caller Creates a Duplex Request\r\n\r\nWhen you do this you can’t also specify a request body.\r\n\r\n```\r\nRequest request = new Request.Builder()\r\n    .url(\"https://api.github.com/markdown/raw\")\r\n    .duplexPost(...)\r\n    .build();\r\n```\r\n\r\nThe returned request has a null body, and returns `true` from a new `isDuplex()` method.\r\n\r\n### Response Has An Outbound Stream\r\n\r\nThis class is structured like a `ResponseBody` that does blocking writes instead of blocking reads.\r\n\r\n```\r\ninterface HttpSink extends Closeable {\r\n  BufferedSink sink();\r\n  void headers(Headers headers);\r\n}\r\n```\r\n\r\nIt’s exposed on the `ResponseBody` and you can access it after you `execute()` or in the callback to `enqueue()`.\r\n\r\n```\r\ntry (Response response = client.newCall(request).execute()) {\r\n  HttpSink sink = response.duplexSink();\r\n  ...\r\n}\r\n```\r\n\r\nI’m using the word `HttpSink` here instead of `DuplexRequestBody` because I think it might be possible to use the same type in MockWebServer when sending these things.\r\n\r\n### Response Has Trailer Callbacks\r\n\r\nThere’s an interface to get callbacks when trailers arrive and a mechanism to register it on the response body.\r\n\r\n```\r\ninterface HeadersListener {\r\n  void onHeaders(Headers headers);\r\n}\r\n```\r\n\r\n```\r\nresponse.body().headersListener(new HeadersListener() {\r\n  @Override public void onHeaders(Headers headers) {\r\n    System.out.println(headers);\r\n  }\r\n});\r\n```\r\n\r\nI’d like to avoid having the word “trailers” in the API because the HTTP/2 wire protocol allows ’em to come anywhere in the stream. Also it would be bad to have a variable called `trailers` with a type of `Headers`.\r\n\r\n### onHeaders Arrives On The User’s Thread\r\n\r\nI’d like to keep response reading single-threaded, but response readers get both bytes (via the `BufferedSource`) and headers. Rather than calling back with headers on another thread, we call back with headers while doing a blocking read of the response body.\r\n\r\nGiven this code:\r\n\r\n```\r\nresponse.body().headersListener(new HeadersListener() {\r\n  @Override public void onHeaders(Headers headers) {\r\n    System.out.println(Thread.currentThread() + \"/headers: \" + headers);\r\n  }\r\n});\r\nSystem.out.println(Thread.currentThread() + \"/body: \" + response.body().readUtf8Line());\r\nSystem.out.println(Thread.currentThread() + \"/body: \" + response.body().readUtf8Line());\r\n```\r\n\r\nIt could print something like this:\r\n\r\n```\r\nmain/body: hello\r\nmain/headers: grpc-status: 0\r\nmain/body: world\r\n```\r\n\r\nWith this design you’ll not be notified of trailers unless you make a blocking call into the stream! Most of our code reads streams to the end so I don’t think this will be a problem.\r\n\r\n# Etc.\r\n\r\n### Interceptors Mostly Work\r\n\r\nInterceptors will need to know to look for the response’s duplex sink instead of the request body. But if they do, they’ll be able to do interesting things like log or encrypt or compress that data. They need to just build a `HttpSink` decorator.\r\n\r\n### Need a Solution For ‘Early’ Data\r\n\r\nThis design doesn’t have a place for the client to send messages to the server until it’s received the response headers. I’m tempted to allow these to keep their request body, and to use it as a prefix to the duplex stream. The biggest drawback of this is that it makes things more complex. For example, that early data’s content length is irrelevant, but still present and confusing. It would miserably fail on things like client-side gzip interceptors, for example.\r\n\r\nThat might be worthwhile regardless; the performance consequences are pretty huge.\r\n\r\n### Blocking Still Sucks\r\n\r\nThis API presumes a lot of threads blocking on reads. Would be nice to have a mechanism in Okio. I reach for coroutines but actually wiring that up is nontrivial.\r\n\r\n### Retrofit\r\n\r\nIt’s [madness](https://github.com/square/misk/blob/master/misk/src/main/kotlin/misk/grpc/GrpcClient.kt) to set gRPC headers and encode gRPC messages directly via OkHttp. Instead we need a nice Retrofit layer on top. Retrofit can accept the complexity of mapping a bytestream of framed gRPC messages into an `Observable<Taco>` or the framework du jour."},"comment":{"url":"https://api.github.com/repos/square/okhttp/issues/comments/445948634","html_url":"https://github.com/square/okhttp/issues/4267#issuecomment-445948634","issue_url":"https://api.github.com/repos/square/okhttp/issues/4267","id":445948634,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NTk0ODYzNA==","user":{"login":"swankjesse","id":133019,"node_id":"MDQ6VXNlcjEzMzAxOQ==","avatar_url":"https://avatars3.githubusercontent.com/u/133019?v=4","gravatar_id":"","url":"https://api.github.com/users/swankjesse","html_url":"https://github.com/swankjesse","followers_url":"https://api.github.com/users/swankjesse/followers","following_url":"https://api.github.com/users/swankjesse/following{/other_user}","gists_url":"https://api.github.com/users/swankjesse/gists{/gist_id}","starred_url":"https://api.github.com/users/swankjesse/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swankjesse/subscriptions","organizations_url":"https://api.github.com/users/swankjesse/orgs","repos_url":"https://api.github.com/users/swankjesse/repos","events_url":"https://api.github.com/users/swankjesse/events{/privacy}","received_events_url":"https://api.github.com/users/swankjesse/received_events","type":"User","site_admin":false},"created_at":"2018-12-10T19:50:01Z","updated_at":"2018-12-10T19:50:01Z","author_association":"MEMBER","body":"I’ve been working with @oldergod on implementing gRPC on OkHttp. We’ve learned that although the HTTP/2 transport layer permits multiple headers + data frames to be interleaved, the HTTP+HTTP/2 layering permits headers only before the data (headers) and after the data (trailers).\r\n\r\nFor HTTP requests there are no trailers after the request body.\r\n\r\nBecause duplex exists for gRPC and no other reason, we’re going to change our APIs to support that use case only. In particular, we need to:\r\n\r\n1. Add an API for MockWebServer to enqueue the end-of-stream trailers. If trailers are to be provided they must be provided before the call to ResponseBody.close().\r\n2. Add an API for OkHttp to collect the end-of-stream trailers. These may only be collected after the response body stream has been exhausted.\r\n\r\nWe may also want to consider an API for OkHttp to write the request body without our `RequestBody` callback. That API is inconvenient when streaming an arbitrarily-large request."}}
{"actor":{"display_login":"swankjesse"}}