ComposedPhysicalExtensionCodec should not rely on positional encoding
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Is your feature request related to a problem or challenge?
ComposedPhysicalExtensionCodec should identify codecs by stable ID instead of list position
`ComposedPhysicalExtensionCodec` records the position of each codec in the `DataEncoderTuple`:
```rust
struct DataEncoderTuple {
pub encoder_position: u32,
pub blob: Vec,
}
The problem is that if someone adds, removes, or reorders a codec, you get really confusing error messages. For example:
1. A writer with [CodecA, CodecB] encodes protobuf `A` and specifies `encoder_position: 0`
2. A reader with [CodecB, CodecA] decodes the proto using `encoder_position: 0` and errors with "CodecB cannot decode message"
```
It's hard to see that this was caused a breaking protocol change. Since both codecs are present, it doesn't seem like a breaking change, but it is.
### Describe the solution you'd like
Maybe we can introduce an id to look up encoders:
```rust
struct DataEncoderTuple {
// Retained for decoding payloads written by older versions.
pub encoder_position: u32,
pub blob: Vec,
pub codec_id: Option,
}
Decoding would:
1. Use codec_id when present.
2. Return an explicit error containing the unknown identifier when it is not registered.
3. Fall back to encoder_position for legacy payloads.
```
### Describe alternatives you've considered
_No response_
### Additional context
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.