Skip to content

Commit da90f62

Browse files
authored
Decode of nil data (#8179)
## What changed? Don't catch `Data: nil` in test; let it fall through to decoder. The decoder will return an error. An error is the better choice than a `nil` response since that signals to the user that the decoded data is usable/valid. ## Why? Follow-up to #8111; an internal test expects an error instead of `nil`. ## How did you test it? - [ ] built - [ ] run locally and tested manually - [ ] covered by existing tests - [x] added new unit test(s) - [ ] added new functional test(s) ## Potential risks Hard to believe that returning nil and using un-decoded data is a good/valid alternative.
1 parent b8497fa commit da90f62

File tree

2 files changed

+10
-3
lines changed

2 files changed

+10
-3
lines changed

common/persistence/serialization/codec.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ func Decode(data *commonpb.DataBlob, result proto.Message) error {
4949
if data == nil {
5050
return NewDeserializationError(enumspb.ENCODING_TYPE_UNSPECIFIED, errors.New("cannot decode nil"))
5151
}
52-
if data.Data == nil {
53-
return nil
54-
}
5552

5653
switch data.EncodingType {
5754
case enumspb.ENCODING_TYPE_JSON:

common/persistence/serialization/codec_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ func TestProtoDecode(t *testing.T) {
4242
assert.Contains(t, err.Error(), "cannot decode nil")
4343
})
4444

45+
t.Run("empty data blob", func(t *testing.T) {
46+
blob := &commonpb.DataBlob{}
47+
48+
var result persistencespb.ShardInfo
49+
err := Decode(blob, &result)
50+
require.Error(t, err)
51+
assert.IsType(t, &UnknownEncodingTypeError{}, err)
52+
assert.Contains(t, err.Error(), "unknown or unsupported encoding type Unspecified")
53+
})
54+
4555
t.Run("nil data field", func(t *testing.T) {
4656
blob := &commonpb.DataBlob{
4757
Data: nil,

0 commit comments

Comments
 (0)