fix(ws): install a rustls crypto provider so the TLS connect does not panic - #80
Open
mkzung wants to merge 1 commit into
Open
fix(ws): install a rustls crypto provider so the TLS connect does not panic#80mkzung wants to merge 1 commit into
mkzung wants to merge 1 commit into
Conversation
mkzung
force-pushed
the
fix/rustls-crypto-provider
branch
from
July 18, 2026 12:28
a2cd064 to
67e8a4c
Compare
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
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.
Fixes #57.
connect_asyncresolves the process-level rustlsCryptoProviderfrom crate features, which panics when more than one provider is present in the dependency graph. That is the default situation here,aws-lc-rsandringare both in Cargo.lock already, so the first TLS connect panics.The panic happens inside the detached
tokio::spawninConnectionManager::new, and itsJoinHandleis dropped, so nothing surfaces. The manager keeps reporting the last state it reached,Connecting, and every subscription stream hangs with no error and no data.Two changes:
ensure_crypto_providerinstalls a default provider if the process does not already have one, so the ambiguity never comes up. An application that has installed its own provider is left alone, I checked that by installingringfrom the test app first and confirming the guard steps aside.The connection loop is wrapped in
catch_unwindand the connection is reported asDisconnectedif it panics. Without that, any panic in that loop is swallowed and becomes a silent hang, which is what made this one hard to see.Against a live market, before:
and after:
REST is not affected. reqwest uses the same rustls version but selects a provider explicitly, and I confirmed it completes the handshake in the same process where tungstenite panics. So this only needs fixing on the websocket path.
ConnectionManageris the only place the SDK opens a TLS connection, so the one fix covers both the clob ws client and rtds.cargo test --features clob,ws,tracingpasses, 388 tests, no failures.cargo fmt --checkis clean and clippy reports the same findings as on main.Two things I ran into while reproducing this, both separate from the fix and not in this PR:
cargo test --features clob,wsdoes not run any tests at all. It fails to build nine of the examples, because they importtracingbut theirrequired-featuresdoes not include it (error[E0432]: unresolved import tracing). It fails the same way on main, so it is not from this change, but it does mean that feature combination has no working test command right now. The affected examples are async, authenticated, aws_authenticated, builder_authenticated, websocket_orderbook, websocket_user, websocket_unsubscribe, rfq_quotes and rfq_requests.That is also why the repro command quoted in the issue,
cargo run --example websocket_orderbook --features clob,ws, does not compile. Addingtracingto the feature list is the workaround.Happy to send a follow-up for the
required-featuresgap if you want it.I used an AI assistant while working on this. I built and ran everything myself, including the live-market check above, and I understand the change.
Note
Medium Risk
Touches TLS/crypto initialization for all WebSocket and RTDS connections; behavior is narrowly scoped but incorrect provider choice could affect handshake compatibility in edge deployments.
Overview
Fixes WebSocket TLS setup when the process has multiple rustls crypto providers in the dependency graph (e.g.
aws-lc-rsandring), which previously causedconnect_asyncto panic on first connect.Adds
ensure_crypto_provider, called when creating aConnectionManager, to installaws-lc-rsas the default only if none is set yet—apps that already chose a provider are unchanged.rustlsis wired in as an optional dependency for thewsandrtdsfeatures.Wraps the spawned connection loop in
catch_unwindso a panic in that detached task sets state toDisconnected(and logs withtracingwhen enabled) instead of leaving clients stuck inConnectingwith no data or error.Includes a unit test that
ensure_crypto_providerleaves a default provider installed.Reviewed by Cursor Bugbot for commit 67e8a4c. Bugbot is set up for automated code reviews on this repo. Configure here.