fix: omit protobuf data for events without payload - #1286
Conversation
Signed-off-by: immanuwell <pchpr.00@list.ru>
|
@immanuwell thanks. Fix seems ok but I have a question, why would json Marshal() ever return bad json like that? It doesn't seem possible to me even if the data were nil |
|
Since we can't control all of the messages we might receive it feels like we might want to also add protection against receiving these nil port but messages, no? |
kunalworldwide
left a comment
There was a problem hiding this comment.
Good fix. Without this, ToProto always sets Data to a BinaryData wrapper even when the event has no payload. That empty BinaryData{BinaryData: nil} gets serialized as a zero-length bytes field in protobuf, which on the Unmarshal side creates an empty []byte{} instead of nil — and when that's subsequently marshalled to JSON, you get "data_base64":"" instead of no data field at all, producing invalid JSON for events like heartbeats that intentionally have no payload.
The guard on DataContentType == ContentTypeProtobuf is also correct — no point trying to wrap nil data into an Any message.
Test is thorough: roundtrips through protobuf marshal/unmarshal, verifies DataEncoded is nil, then confirms the JSON output is clean and re-parseable.
LGTM.
Fixes #1193
protobuf marshal always sets
data, even when the event has no payload.after a protobuf round trip that becomes
DataEncoded=[]byte{}, then JSON marshal emits busted JSON:{"specversion":"1.0","id":"id","source":"source","type":"type","datacontenttype":"application/json","data":}Repro on
main:This patch skips protobuf
datawhene.Data() == nil, so no payload stays no payload. small fix, but pretty real.Tests:
go test ./...inbinding/format/protobuf/v2go test ./...inv2