xmtp / libxmtp

LibXMTP is a shared library encapsulating the core functionality of the XMTP messaging protocol, such as cryptography, networking, and language bindings.
MIT License
47 stars 19 forks source link

replace `reqwest` with `hyper`/`hyper-util` for `xmtp_api_http` in `create_grpc_stream` #997

Open insipx opened 2 months ago

insipx commented 2 months ago

we have a crate, xmtp_api_http that is a drop-in replacement for xmtp_api_grpc in the case that we need to use xmtp_mls on the web. This crate is generally working, and all tests in xmtp_mls pass.

Reqwest keeps a connection pool alive when using their library, in order to re-use http connections. For our streaming POST requests in xmtp_api_http, we expect the connection to stay alive for the entirety of the request. Reqwest actually spawns a future when the request is sent to watch the request, and re-add the connection to the pool once the request ends. When used with a single-threaded executor, this blocks execution from progressing, since the POST request we're using is long-lived and doesn't end, the two futures mutually keep each other alive and prevent the other from finishing.

We can fix this by using hyper (the lower-level library reqwest is built upon), and crafting the POST request manually & using a separate connection from reqwest. This will allow our streams to be used in current_thread and prevent blocking the executor.

Relevant tests: test_stream_messages, test_stream_conversations

Note: WebAssembly uses a different connection pooling/stream strategy than native reqwest, so it's to be seen if this is a blocker for webassembly support.

rel: https://github.com/seanmonstar/reqwest/issues/500

insipx commented 2 months ago

Just a bit more context:

The tests I'm talking about are here: https://github.com/xmtp/libxmtp/blob/1cabe0ab0429d38bdcabec67aeb8f2cb68d0c604/xmtp_mls/src/subscriptions.rs#L399

using hyper for the streams should allow the test linked and the one below that to complete without tokio::spawning a new task for the stream