Skip to content

fix(sdk): TcpTransport.send_push raises on OP_ERROR_RESPONSE instead of returning the error envelope#130

Merged
petrpan26 merged 1 commit into
mainfrom
fix/sdk-tcp-send-push-opcode-check
May 14, 2026
Merged

fix(sdk): TcpTransport.send_push raises on OP_ERROR_RESPONSE instead of returning the error envelope#130
petrpan26 merged 1 commit into
mainfrom
fix/sdk-tcp-send-push-opcode-check

Conversation

@petrpan26
Copy link
Copy Markdown
Contributor

Summary

`TcpTransport.send_push` was blindly JSON-decoding the response frame and returning the dict to user code. When the server emitted `OP_ERROR_RESPONSE` (e.g. `invalid_event` on a type mismatch), the error envelope was returned as a regular dict — no exception. Embed mode defaults to TCP, so fire-and-forget pushes silently `/dev/null` on validation failure.

PR #120 documented this and locked the buggy behaviour with `test_type_error_at_push.py`. This PR fixes the bug and flips those tests to assert the correct contract.

Fix in `python/beava/_transport.py`

`send_push` (lines 443-490) now mirrors `send_get`:

  • After `read_frame`, check `frame.op != OP_PUSH` (the success-echo opcode — server reuses `OP_PUSH`, not a separate `OP_PUSH_RESPONSE`).
  • If `OP_ERROR_RESPONSE`: parse the JSON body with try/except guards and `raise RegistrationError(code=err_body["error"]["code"], message=...)`.
  • Fallback: `"unparseable_error"` for bad bytes / `"unexpected_frame"` for missing code.

Docstring expanded with success/error wire shapes and `Raises:` section.

Test flips in `python/tests/test_type_error_at_push.py`

  • 5 type-mismatch tests flipped from `_assert_push_error(...)` (which asserted the buggy return-dict shape) to `with pytest.raises(RegistrationError) as exc_info: ...; assert exc_info.value.code == ""`.
  • Test #2 (float→int silent accept) unchanged — server still legitimately accepts that case via numeric I64↔F64 compat, returns ack_lsn.
  • Added 2 new tests with in-process TCP mock server:
    • `test_push_response_unexpected_opcode_raises` — server replies with bogus opcode → `RegistrationError(code="unexpected_frame")`.
    • `test_push_error_response_with_unparseable_body_raises` — non-JSON error body still raises cleanly (no crash).

Test plan

  • Before fix: 6/6 passed by asserting the buggy return shape.
  • After fix: 8/8 passed (`pytest python/tests/test_type_error_at_push.py`, 21.26s).
  • Re-run against current `main` (`b20d2b83`) — clean.
  • `ruff check` clean.

…of returning error envelope

send_push blindly JSON-decoded the response frame and returned the dict
regardless of opcode. On OP_ERROR_RESPONSE (invalid_event, unknown_field_v0,
etc.) the error envelope {"error":{"code":...}} came back as a normal
return value — fire-and-forget callers silently dropped events on validation
failure. Embed mode defaults to TCP, so this hit the canonical docs path.

Mirror send_get: if frame.op != OP_PUSH, parse the body and raise
RegistrationError with the server code (or 'unparseable_error' /
'unexpected_frame' as fallback).

Flip the 5 type-mismatch tests in test_type_error_at_push.py from asserting
the buggy {"error": ...} return shape to pytest.raises(RegistrationError);
add 2 new tests covering unexpected opcode and unparseable error body via an
in-process TCP mock.
@petrpan26 petrpan26 merged commit 9571986 into main May 14, 2026
12 checks passed
@petrpan26 petrpan26 deleted the fix/sdk-tcp-send-push-opcode-check branch May 14, 2026 12:50
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.

1 participant