Repository navigation
feat(core): expose response metadata and socket payload types - #826
Conversation
|
GregHolmes
changed the title
GregHolmes
changed the title
GregHolmes
changed the title
GregHolmes
changed the title
dg-coreylweathers
left a comment
dg-coreylweathers
left a comment
There was a problem hiding this comment.
Reran this in Docker (python:3.12) at 1e8c95b. Mostly good. Requesting changes for S2, S3 and N1. S1 is a Fern issue so no fix needed here, N2/N3 are FYI.
What I checked
- Do unknown server frames come back as raw dicts, not None? Yes for listen v1/v2, speak v2, agent v1 (sync iter, async iter, recv(), start_listening). I ran a local ws server sending {"type":"BrandNewServerEvent"}, {"foo":"bar"}, {"type":"Results"}. No None, no crash.
- Tests actually ran? Yes. 1147 passed / 4 skipped with wiremock up (main is 1138/4). Heads up: pytest dies at startup with no docker in the container, so I ran wiremock as a sibling container and set WIREMOCK_URL. mypy src/ tests/typecheck is clean.
- Greg's hand patch (typing.Dict[str, Any] added to the 5 socket_client.py unions): it is type-only, runtime already works from Fern's own unchecked_base_model change. It survives regens because those 5 files are already in .fernignore. Correct for the 4 sockets above.
- Version: stays 7.17.0, matches the released tag. Good.
To fix (S2, S3, N1) / Fern issue (S1) / FYI (N2, N3)
[S1] speak.v1 doesn't give a dict. An unknown frame there comes back as SpeakV1Flushed (type="BrandNewServerEvent"), and {"foo":"bar"} too. Same on main, so not new, but the PR body says unknown handling is restored and the new Dict member on that union is never reached. This one is a Fern issue (undiscriminated-union casting in the generator), not something to fix in this PR. Just raise it with Fern alongside the dict-member ask, and don't block on it.
[S2] No runtime test for the socket fallback. The new coverage is a typecheck file plus 4 lines in test_core_utilities_coverage.py. Please add one test per socket client that feeds an unknown frame and asserts isinstance(msg, dict). That would have caught S1.
[S3] RemoteProtocolError no longer retries by default (retry_remote_protocol_errors opt-in). Reasonable, but it's a behavior change for anyone relying on the old retry, and the title is feat(regen) so release-please writes it up as a plain minor. Can we get an explicit line in the release notes?
[N1] AGENTS.md: the socket_client.py entries say "same" and don't mention the new Dict member. Add one line, and add it to the list of things to raise with Fern (generator should emit the dict member itself so the freeze can shrink).
[N2] Typed users may see new mypy errors: the sockets' iter/aiter are now annotated (were unannotated), and the union is bytes | models | dict. Anyone doing m.type in a loop needs an isinstance. Worth a release note line too.
[N3] http_client.py retry recursion no longer passes optional_body (Greg's patch was dropped, correctly, since the file isn't frozen). Retried optional-body calls fall back to the default. I couldn't find a case where it changes the wire, just flagging it's generator behavior now.
Re S3: I'd keep it a minor with a release note rather than feat!.
GregHolmes
changed the title
|
@dg-coreylweathers Addressed the review items:
Focused tests, mypy, and the Python 3.10-3.13 CI matrix pass. |
dg-coreylweathers
left a comment
dg-coreylweathers
left a comment
There was a problem hiding this comment.
Requesting changes on a few release-notes items. The code itself looks good: I ran the unknown-frame handling against a local WebSocket server and mypy, ruff and pytest pass in Docker (3.12).
Retry default is a behavior change shipping as a patch (S4). RemoteProtocolError no longer retries by default. The commits are now fix(...) with no breaking marker, so Release Please would propose 7.17.1. Please make it a minor (or mark it breaking) so it doesn't land in a patch.
The release note isn't in the repo yet (S3). The note for the retry change is planned for the Release Please PR, which doesn't exist yet, so nothing in the repo records it. Please add the line here, for example: "RemoteProtocolError no longer retries by default; opt in with request_options={'retry_remote_protocol_errors': True}".
The title says more than the PR delivers (S5). Speak V1 still turns unknown frames into SpeakV1Flushed (the Fern bug in fern#18824, already disclosed and pinned by the xfail test). Please word the title so it doesn't promise unknown-message preservation for Speak V1.
Typed users may see new mypy errors (N2). The new union members (including bytes on Listen v1/v2) can surface new errors in typed callers. It's in AGENTS.md but users won't see that, so please add a short user-facing release note.
Everything else I checked held up.
GregHolmes
changed the title
|
@dg-coreylweathers Follow-up on the review items:
CI remains green across Python 3.10-3.13. |
dg-coreylweathers
left a comment
dg-coreylweathers
left a comment
There was a problem hiding this comment.
Approving. Greg's replies cover the asks. The head is the same 6ae6912 I already ran, and in Docker (3.12) the socket fallback, retry and core coverage tests pass, mypy is clean and ruff is clean. The Speak V1 case is an xfail pinned to fern#18824, as agreed.
- S2:
tests/custom/test_socket_unknown_fallback.pyfeeds unknown frames to each socket client and covers binary Listen frames. - S4 and S5: the title is
feat(core), so release-please proposes a minor, and it no longer promises unknown-frame preservation for Speak V1. - S3 and N2: the retry opt-in line and the typing change are in the PR body, which becomes the squash body. I'm fine leaving CHANGELOG.md to Release Please as long as that body line makes it into the squashed commit.
Not tested here: the wiremock wire suite, which needs a sibling container. CI shows green on 3.10 to 3.13.
🤖 I have created a release *beep* *boop* --- ## [7.18.0](v7.17.0...v7.18.0) (2026-11-10) ### Features * **core:** Preserve unknown Agent V1, Listen V1/V2, and Speak V2 WebSocket messages as raw dictionary payloads instead of dropping them; Listen V1/V2 binary frames are now typed. Responses from `with_raw_response()` expose the underlying `httpx.Response` as `.response`. To avoid replaying a request the server may already have applied, `httpx.RemoteProtocolError` is no longer retried by default; opt in for safe requests with `request_options={"retry_remote_protocol_errors": True}`. ([#826](#826)) ([3b3679c](3b3679c)) ## Highlights - Unknown Agent V1, Listen V1/V2, and Speak V2 WebSocket frames now reach applications as raw dictionary payloads when the SDK does not recognize their variant. - `with_raw_response()` wrappers now expose the underlying `httpx.Response` through `.response`. - `httpx.RemoteProtocolError` retries are opt-in through `request_options={"retry_remote_protocol_errors": True}` to prevent accidental replay of a request that the server may have already applied. --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Greg Holmes <greg.holmes@deepgram.com>
Summary
Regenerate the Python SDK with
fernapi/fern-python-sdk5.39.3.None. Agent V1, Listen V1/V2, and Speak V2 response unions type those fallback values as dictionaries; Listen V1/V2 also correctly type their existing binary-frame paths. This supersedes the unmerged workaround in fix(listen): preserve unknown v1 and v2 websocket responses #804.SpeakV1Flushedbecause of a Fern undiscriminated-union defect.with_raw_response()now expose the underlyinghttpx.Responseas.response.httpx.RemoteProtocolErrorno longer retries by default because the server may already have applied the request. For safely replayable operations, opt in withrequest_options={"retry_remote_protocol_errors": True}.pydantic_utilities.pymypy workaround because Fern now handles itsTypeAdapterdiagnostic.api_key=Nonebehavior in the custom client after Fern moved environment lookup intoBaseClient..gitignorecoverage-artifact rules and removed all transient.bakfiles.Upstream Follow-up
Closes
Validation
poetry run pytest -rP .- 1147 passed, 4 skippedpoetry run ruff check src tests/wire tests/custom/test_http_retry_coverage.py tests/typecheck/compat_aliases.pypoetry run mypy src/ tests/typecheckmanage.v1.projects.list()- HTTP 200poetry run ruff check .continues to report existing violations in manual examples andtests/custom/test_transport.py, outside this regeneration.