Skip to content

Surface replayed errors faithfully and key cassettes by a text method - #11

Merged
tboser merged 3 commits into
mainfrom
fix-error-fidelity-and-portability
Aug 20, 2026
Merged

tboser merged 3 commits into
mainfrom
fix-error-fidelity-and-portability

Conversation

@tboser

@tboser tboser commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Both bugs surfaced by #10, fixed test-first. Commit 1 adds the tests and is red; commit 2 makes them green.

1. Failed streams were reported as successful ones

A failing server-streaming or bidi RPC raises grpc.RpcError on a plain gRPC channel. Through the sync interceptors it returned an empty stream while recording the correct status code, so a call that should fail a test passed it. The async interceptors raised correctly, so the two paths disagreed with each other as well as with gRPC.

plain gRPC:      raised INVALID_ARGUMENT
grpcvcr record:  returned normally, 0 messages
   recorded code:  INVALID_ARGUMENT
grpcvcr replay:  returned normally, 0 messages

_FakeStreamingCall.__next__ never consulted self._code. It now delivers the messages that arrived before the failure, then raises, which is what gRPC does.

Replayed unary errors had a related problem: they raised a bare grpc.RpcError carrying no status, even though docs/examples/advanced.md already showed assert e.code() == grpc.StatusCode.NOT_FOUND on a replayed error. The docs were right and the code was wrong. Both fakes now raise ReplayedRpcError, which is a grpc.RpcError and a grpc.Call the way gRPC's own error object is, so the status reads off the exception.

2. Cassettes were not portable between channel kinds

client_call_details.method arrives as str on the sync path and bytes on the async one, so the recorded key differed and MethodMatcher never matched across them. Async-recorded cassettes also stored the method as a binary blob, which defeats reading the file:

  - request:
      method: !!binary |
        L3Rlc3QuVGVzdFNlcnZpY2UvR2V0VXNlcg==

normalize_method decodes it at the four aio entry points. Cassettes now round-trip in both directions and the method is written as text.

Red, then green

Commit 1 (7b499d5), tests only:

8 failed, 4 passed

The four that pass are the controls: the async path already raised, the sync path already wrote a text method, and successful streams still complete.

Commit 2 (262598b), with the fix:

181 passed
Required test coverage of 100.0% reached. Total coverage: 100.00%

Notes

  • Two tests from Make the coverage gate measure the whole library #10 asserted that a failed stream ends quietly. They pinned the defect, so they move to the corrected contract here.
  • ReplayedRpcError implements exactly what grpc.Call declares abstract, plus __str__. It is not exported: callers catch grpc.RpcError.
  • The error-handling guide gains the streaming case, which it did not cover.

make all green, mkdocs build --strict clean.

tboser added 2 commits August 20, 2026 19:22
Two defects, pinned before fixing.

A failed server-streaming or bidi RPC raises `grpc.RpcError` on a plain gRPC
channel. Through the sync interceptors it returns an empty stream instead, while
recording the correct status code, so a call that should fail a test passes it.
The async interceptors raise correctly, so the two paths disagree with each
other as well as with gRPC. Replayed unary errors raise a bare `grpc.RpcError`
carrying no status, which the documented example already assumes it can read.

Separately, `client_call_details.method` arrives as `str` on the sync path and
`bytes` on the async one. The recorded key differs between them, so cassettes do
not replay across channel kinds, and an async-recorded cassette stores the
method as a `!!binary` blob instead of readable text.
`_FakeStreamingCall.__next__` never consulted the recorded status, so a failed
stream ended as if it had succeeded. It now delivers the messages that arrived
before the failure and then raises, matching what gRPC does and what the async
path already did.

The error raised is a `ReplayedRpcError`, which is both a `grpc.RpcError` and a
`grpc.Call` the way gRPC's own is, so `code()` and `details()` are readable off
the exception. The unary fake raises the same object in place of the bare
`grpc.RpcError` it used to raise, which carried no status at all despite the
documented example reading one from it.

`normalize_method` decodes the method the async path receives as bytes, so both
channel kinds record the same key. Cassettes now replay across them, and an
async-recorded file holds a readable method instead of a `!!binary` blob.

The two tests that asserted a failed stream ends quietly are updated to the
corrected contract, and the error-handling guide gains the streaming case.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

It surfaces while recording a failure as well as while replaying one, so
`_RecordedRpcError` describes it where `ReplayedRpcError` did not. The underscore
matches the other fakes in this module and marks it as not a public type: the
sync path raises this and the async path raises `aio.AioRpcError`, so callers
catch `grpc.RpcError`, which covers both. The docstring records why the
stand-in exists and where the asymmetry comes from.
@tboser

tboser commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

Follow-up on the two review points.

The tests that pinned the bug are updated to the corrected contract in 262598b, not deleted: they still assert what lands in the cassette, which the new tests do not cover, so only the client-visible expectation changed.

On exporting the error type, I went to make it public and found it should not be. The two paths raise different types:

sync  -> grpcvcr.interceptors._base.ReplayedRpcError
async -> grpc.aio._call.AioRpcError

That is correct rather than a defect: gRPC ships AioRpcError for async, and its sync equivalent _InactiveRpcError is private, so only the sync side needs a stand-in. But exporting the stand-in would invite except ReplayedRpcError, which silently catches nothing on an async call. except grpc.RpcError covers both and is what the guide already shows.

So 9b09963 goes the other way: the class is now _RecordedRpcError, underscored to match the other fakes in that module, with the asymmetry and the reason for the stand-in written into its docstring. The name also fixes an inaccuracy, since it is raised while recording a failure and not only while replaying one.

Happy to export it instead if you would rather have it public; it is a one-line change plus a docs entry.

@tboser
tboser merged commit 20003b6 into main Aug 20, 2026
6 checks passed
@tboser
tboser deleted the fix-error-fidelity-and-portability branch August 20, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants