fix(http): honor custom reqwest Client on WebSocket connect - #333
Open
SebTardif wants to merge 1 commit into
Open
fix(http): honor custom reqwest Client on WebSocket connect#333SebTardif wants to merge 1 commit into
SebTardif wants to merge 1 commit into
Conversation
run_ws discarded the configured Client and called async-tungstenite connect_async with only the URL. HttpClient::with_client and with_endpoint_and_client therefore ignored timeout, default headers, proxy, and TLS on ws:// and wss://. Perform the handshake with the reqwest Client, then wrap the upgraded stream as a tungstenite WebSocket. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HttpClient::with_clientandwith_endpoint_and_clienttake a configuredreqwest::Client. HTTP/SSE uses that client. The WebSocket path discarded it and calledasync_tungstenite::tokio::connect_asyncwith only the URL.A caller that sets timeout, default headers, proxy, or custom TLS on the
reqwest::Clienttherefore gets none of those settings onws://orwss://.This has been true since the HTTP/WebSocket transport landed in #162.
Change
run_wsnow performs the WebSocket handshake with the configuredreqwest::Client(HTTP/1.1 upgrade), then wraps the upgraded stream inasync-tungstenite. The client's timeout, default headers, proxy, and TLS apply to the connect.Tests
websocket_with_client_sends_default_headers: publicHttpClient::with_clienton a localws://server. The handshake includes a default header set on thereqwest::Client.websocket_with_client_honors_request_timeout: the same API against a listener that never accepts. A 200ms client timeout fails the handshake instead of hanging.Red on unfixed
main: the header is missing, and the timeout test hits the 1s outer timeout (Elapsed). Both pass after the change.Does not overlap with #315 (server-side WebSocket frame limits), #328, #322, or #308.