jiengup opened a new pull request, #3751:
URL: https://github.com/apache/iggy/pull/3751

   variants
   
   Server now serializes the fields of data-bearing error variants (e.g. 
PartitionNotFound, StreamIdNotFound) into a binary payload and sends it 
alongside the error code. Client transports (QUIC, TCP, WebSocket, VSR) read 
this payload and reconstruct the full error, enabling contextual error messages 
on the client side. The Python SDK e2e test is updated to match the new dynamic 
IDs.
   
   ## Which issue does this PR address?
   
   Closes #3735
   
   ## Rationale
   Due to `&[]` payload in `send_error_response<T>` in 
`core/server/src/sender/mod.rs` here:
   
   
https://github.com/apache/iggy/blob/d9635b6d71dacbe6be1702a0723df50f5574b789/core/server/src/sender/mod.rs#L199-L207
   
   
   Error payloads (`partition_id`, `topic_id`, `stream_id`, etc.) were 
systematically dropped on the wire for
   TCP, QUIC, WebSocket, and VSR transports. Clients received fielded variants 
filled with `Default` values,
   producing byte-identical errors for completely different requests.
   
   ```
   Server: PartitionNotFound(5, topic_42, stream_1)
    └─ send_error_response → [3007_u32] + empty body Wire: [0xDF 0x0B 0x00 
0x00] [0x00 0x00 0x00 0x00] Client: from_code(3007) → FromRepr → Default fill
    └─ PartitionNotFound(0, Identifier(0), Identifier(0))
   ```
   
   The HTTP transport was unaffected (`error.to_string()` serialized in JSON), 
proving the data existed
   and was merely not transported.
   
   ## What changed?
   
   **Server** (`send_error_response`) now serializes the error’s fields into a 
structured binary payload
   instead of an empty body. **Client** (TCP, QUIC, WebSocket, VSR) reads the 
body and reconstructs the
   original variant with real field values via `from_code_with_payload`. The 
old empty-body path falls
   back to `from_code` for backward compatibility with pre-fix servers.
   
   A declarative macro (`implement_error_payload!`) generates both 
serialization and deserialization
   from a single list of variant field declarations. An `ErrorPayloadField` 
trait encapsulates
   the wire encoding for `usize`, `u32`, `u64`, `u8`, `u16`, `String`, and 
`Identifier`.
   90 data-bearing variants are covered; unit variants and unsupported custom 
types produce empty payloads.
   
   ### Files
   
   | File | Change |
   |------|--------|
   | `core/common/src/error/iggy_error.rs` | +342: `ErrorPayloadField` trait, 
`implement_error_payload!` macro, `write_payload`, `from_code_with_payload`, 
~80 new tests |
   | `core/server/src/sender/mod.rs` | `send_error_response` calls 
`write_payload()` instead of sending empty body |
   | `core/sdk/src/tcp/tcp_client.rs` | `handle_response` reads body and calls 
`from_code_with_payload` |
   | `core/sdk/src/quic/quic_client.rs` | Same |
   | `core/sdk/src/websocket/websocket_client.rs` | Same |
   | `core/sdk/src/vsr.rs` | `decode_response` / `decode_response_split` pass 
body to `from_code_with_payload` |
   | `foreign/python/tests/test_message_operations.py` | End-to-end test: 
invalid partition error now asserts real partition ID |
   
   ## Local Execution
   
   - 489 unit tests passed (iggy + iggy_common + server, both non-VSR and VSR 
features)
   - Clippy: 0 warnings
   - Python test syntax verified
   
   ## AI Usage
   
   - Anthropic Claude (via opencode)
   - Architecture design, serialization trait/macro implementation, client 
binding updates
   - All generated code compiled and tested; every line reviewed
   
   ## Discussion
   
   The current approach uses a declarative macro to cover 90 data-bearing 
variants.
   A more elegant impelementation should be considered:
   
   ### Option 1: Proc macro / unified serialization module
   
   A `#[derive(ErrorPayload)]` proc macro would eliminate hand-listing 
variants—new
   field-bearing variants get payload support automatically. The read/write 
logic could
   be further generalized into a `WireField` trait shared with 
`WireEncode`/`WireDecode`
   in `iggy_binary_protocol`, avoiding duplicate serialization infrastructure.
   
   ### Option 2: External crate
   
   Replace the custom `ErrorPayloadField` trait and macro with a third-party 
binary
   serialization crate. `Identifier` already derives `Serialize`/`Deserialize`, 
so
   integration cost is low.
   `
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to