Skip to content

Rust codegen: forward-compatible union deserialization in structs#3414

Open
santiagomed wants to merge 1 commit intoapache:masterfrom
santiagomed:fix/rust-union-forward-compat
Open

Rust codegen: forward-compatible union deserialization in structs#3414
santiagomed wants to merge 1 commit intoapache:masterfrom
santiagomed:fix/rust-union-forward-compat

Conversation

@santiagomed
Copy link
Copy Markdown
Contributor

@santiagomed santiagomed commented Apr 22, 2026

Problem

When a struct contains a union-typed field and the wire data has a union variant (field ID) not present in the local IDL, the generated Rust code fails to deserialize the entire struct — not just the unknown union field.

The root cause: the union's read_from_in_protocol correctly returns Err("received empty union") for unknown variants (fixed in THRIFT-5926 / PR #3336). But the parent struct's generated deserializer propagates this error via ?, causing the whole struct to fail.

This breaks Thrift's forward-compatibility guarantee. When a server adds a new union variant, all clients running an older IDL version fail to parse any message containing that variant — even though the client doesn't need to understand the new variant to process the rest of the struct.

Java, Go, Python, and C++ Thrift all handle this gracefully: unknown union variants are silently skipped, and the struct field is left unset.

Fix

In render_struct_sync_read, detect struct fields whose type is a union. For those fields, wrap the read_from_in_protocol call in a match that catches the "received empty union" error and treats it as None (field absent):

// Before (all fields):
let val = FooUnion::read_from_in_protocol(i_prot)?;
f_N = Some(val);

// After (union fields only):
match FooUnion::read_from_in_protocol(i_prot) {
    Ok(val) => { f_N = Some(val); },
    Err(thrift::Error::Protocol(ref e))
        if e.message.contains("received empty union") => {
        // forward compatibility: unknown union variant skipped
    },
    Err(e) => return Err(e),
}

Non-union fields are unchanged. The union's own TSerializable impl is unchanged — it still returns Err for unknown variants, preserving the contract for callers that read unions directly.

Impact

  • Fixes forward compatibility for all Rust Thrift users with evolving union schemas
  • No changes to the TSerializable trait or the thrift runtime library
  • No changes to the union`s own serialization/deserialization
  • Struct fields with union types that receive unknown variants get None instead of causing parse failure
  • All other errors (malformed data, I/O, multiple union fields) still propagate normally

@mergeable mergeable Bot added the compiler label Apr 22, 2026
@santiagomed santiagomed force-pushed the fix/rust-union-forward-compat branch from 6c16a79 to d3225c2 Compare April 22, 2026 05:19
…n structs

When a struct field has a union type, the generated Rust deserializer
now catches 'received empty union' errors from unknown variants and
treats them as None instead of propagating the error.

Previously, if a Thrift union received a field ID not present in the
local IDL, the union deserializer returned Err('received empty union'),
which cascaded up and caused the entire parent struct deserialization
to fail. This broke forward compatibility.

The fix wraps union field reads in struct deserializers with a match
that catches the specific 'received empty union' error and silently
skips the field. The struct field (already Option<UnionType>) remains
None, matching the behavior as if the field were absent. All other
errors (malformed data, I/O failures) still propagate normally.

Handles both direct union fields and Box<Union> (forward typedef)
fields by resolving the type before generating the method call.

This aligns Rust with Java, Go, Python, and C++ Thrift behavior.
@santiagomed santiagomed force-pushed the fix/rust-union-forward-compat branch from d3225c2 to 61e19a6 Compare April 22, 2026 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant