fix: route Speak V2 websocket through custom transport_factory - #766
Open
mayankbohradev wants to merge 1 commit into
Open
fix: route Speak V2 websocket through custom transport_factory#766mayankbohradev wants to merge 1 commit into
mayankbohradev wants to merge 1 commit into
Conversation
mayankbohradev
requested review from
GregHolmes,
deepgram-kiley and
dg-coreylweathers
as code owners
August 13, 2026 15:09
`_TARGET_MODULES` in transport.py lists the modules that `install_transport()` patches so a user-supplied `transport_factory` replaces the default `websockets` transport. The Speak V2 websocket client shipped in 7.7.0 and binds the same patched symbols, but its two modules were never added to the list. Effect: a caller passing `transport_factory` gets custom-transport routing for Listen v1/v2, Speak v1 and Agent v1, while Speak V2 opens a direct connection to api.deepgram.com. There is no error or warning, so traffic intended to flow through a proxied or custom-hosted transport silently bypasses it. The existing transport tests could not catch this. They iterate `_TARGET_MODULES` itself, so they verify the list is internally consistent but cannot detect a module missing from it -- the missing entry is never iterated. Left as-is, the next regen that adds a websocket client reintroduces the same silent bypass. Changes: - add deepgram.speak.v2.raw_client and deepgram.speak.v2.client to `_TARGET_MODULES` (and correct the now-stale count in the comment) - add a completeness guard that discovers websocket modules from the package source, independently of `_TARGET_MODULES`, and asserts every discovered module is registered Verified: the new guard fails on the unpatched tree naming both Speak V2 modules, and passes after the fix. 319 passed, 1 skipped across tests/custom and tests/utils; `mypy src/` and `mypy tests/typecheck` clean. WireMock-backed tests under tests/wire require Docker and were not run locally.
mayankbohradev
force-pushed
the
fix/speak-v2-transport-factory
branch
from
August 13, 2026 15:23
d6d879e to
67b7341
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.
Summary
A
transport_factorypassed toDeepgramClient/AsyncDeepgramClientis not applied to the Speak V2 websocket client. Speak V2 connects through the defaultwebsocketstransport instead, with no error and no warning.Who this affects
transport_factoryexists so callers can route websocket traffic through their own transport — a proxied connection, a test double, or a custom-hosted endpoint (the.fernignorenotes cite a SageMaker transport as the motivating case).Today a caller who sets it gets custom-transport routing for Listen v1, Listen v2, Speak v1 and Agent v1, while Speak V2 opens a direct connection to
api.deepgram.com. Because the bypass is silent, a caller relying on a custom transport for network-egress or auditing reasons has no signal that part of their traffic isn't going through it.Root cause
install_transport()patches the modules listed in_TARGET_MODULES(src/deepgram/transport.py:32). The Speak V2 websocket client shipped in 7.7.0 and binds the samewebsockets_sync_client/websockets_client_connectsymbols the patcher targets, butdeepgram.speak.v2.raw_clientanddeepgram.speak.v2.clientwere not added to the list.Reproduction on
main— installing a factory and checking whether each module's symbols are shim instances:deepgram.speak.v1.raw_clientdeepgram.speak.v1.clientdeepgram.speak.v2.raw_clientdeepgram.speak.v2.clientdeepgram.listen.v2.raw_clientWhy the existing tests didn't catch it
The transport tests iterate
_TARGET_MODULESitself:That verifies the list is internally consistent, but it can't detect a websocket client missing from the list, because the missing entry is never iterated. So the suite passes whether or not the list is complete.
This is the part worth fixing beyond the two entries: without a completeness check, the next regen that introduces a websocket client reintroduces the same silent bypass.
Changes
src/deepgram/transport.py— adddeepgram.speak.v2.raw_clientanddeepgram.speak.v2.clientto_TARGET_MODULES, positioned after the Speak V1 pair to keep the existing grouping. Also corrects the now-stale count in the comment above the list.tests/custom/test_transport.py— add a completeness guard that discovers websocket modules by scanning the package source, independently of_TARGET_MODULES, and asserts every discovered module is registered. A second small test asserts the scan actually finds a known module, so the guard can't pass vacuously if discovery ever returns nothing.Two deliberate choices in the guard, both easy to change if you'd prefer otherwise:
pkgutil.walk_packages— no import side effects, no dependence onsys.modulesordering, and it still sees a module that fails to import._TARGET_MODULESever intentionally lists a module that doesn't exist yet. Subset targets the case that actually causes the bug: a real websocket client that isn't registered.I kept this to the smallest change that fixes the defect and prevents recurrence. Happy to go further and replace the hardcoded list with runtime discovery if you'd prefer that direction — I left it out to keep the diff reviewable and because it changes behaviour in a file that's deliberately hand-maintained.
Verification
319 passed, 1 skippedacrosstests/customandtests/utils(36/36 intest_transport.py).mypy src/— clean, 882 files.mypy tests/typecheck— clean.main(2 pre-existingwebsocketsdeprecation warnings).Not run locally: the WireMock-backed tests under
tests/wireneed Docker, which wasn't available in my environment, so I haven't executed them. Nothing in this change touches those paths, but flagging it rather than implying a full local run.