Add a http server proxy example#66
Conversation
tomasol
force-pushed
the
http-proxy-example
branch
from
e3aeefa to
2142719
Compare
sunfishcode
left a comment
sunfishcode
left a comment
There was a problem hiding this comment.
Thanks or this example! I agree a proxy is great example to have. I just have a few comments to simpliy the code:
examples/http_server_proxy.rs
Outdated
| // Copy headers from server request to the client request. | ||
| for (key, value) in server_req.headers() { | ||
| client_req = client_req.header(key, value); | ||
| } |
There was a problem hiding this comment.
Instead of copying the headers one at a time, could this do what the http server example does?
There was a problem hiding this comment.
Done in the simplified example.
examples/http_server_proxy.rs
Outdated
| .headers_mut() | ||
| .unwrap() | ||
| .append(key, value.clone()); | ||
| } |
There was a problem hiding this comment.
And similarly, could this avoid copying headers individually?
There was a problem hiding this comment.
Done in the simplified example.
examples/http_server_proxy.rs
Outdated
| let server_req_to_client_req = async { | ||
| let res = copy(server_req.body_mut(), &mut client_request_body).await; | ||
| // TODO: Convert to io error if necessary | ||
| let _ = Client::finish(client_request_body, None); |
There was a problem hiding this comment.
For now, can we map io errors to ErrorVariant::BodyIo error?
There was a problem hiding this comment.
I am constructing a std::io::Error now.
examples/http_server_proxy.rs
Outdated
| let (mut client_request_body, client_resp) = client | ||
| .start_request(client_req) | ||
| .await | ||
| .expect("client.start_request failed"); |
There was a problem hiding this comment.
Using start_request works here, though is a little more verbose than needed for a simple proxy. If you change the request above to use .body(server_req.into_body()), then you can use plain send instead of start_request and manually copying the body.
There was a problem hiding this comment.
Done in the simplified example.
examples/http_server_proxy.rs
Outdated
| } | ||
| // Start sending the server response. | ||
| let server_resp = server_resp.body(BodyForthcoming).unwrap(); | ||
| let mut server_resp = responder.start_response(server_resp); |
There was a problem hiding this comment.
Similar to above, this can do server_resp.body(client_resp.into_body()) and plain respond.
There was a problem hiding this comment.
Done in the simplified example.
|
Thanks for the code review. I wanted to show a streaming example, as it took me a while to get it, but I understand that having something more simple might be useful as well. I have fixed the TODO and created |
tomasol
force-pushed
the
http-proxy-example
branch
from
38fa832 to
0c2b8ab
Compare
|
@pchickey I've attempted to upgrade the streaming proxy example to the latest |
|
That should be |
|
Thanks, that gave me the right direction. I couldn't figure out why the proxy was broken WRT grpc-web, but then I found out it is not forwarding response headers. |
|
Thanks. Last thing: can you please add a |
|
Sure, adding the test. |
3 participants
This is a proposal to add a HTTP proxy example as it seems to be a common use case.