Merge pull request #55 from monzo/fix-marshalling

Fix boolean pointer marshalling

Author: olib963

marshaling.go

@@ -15,24 +15,14 @@ func Marshal(e *Error) *pe.Error {
 		}
 	}
 
-	retryable := &pe.BoolValue{}
-	if e.IsRetryable != nil {
-		retryable.Value = *e.IsRetryable
-	}
-
-	unexpected := &pe.BoolValue{}
-	if e.IsUnexpected != nil {
-		unexpected.Value = *e.IsUnexpected
-	}
-
 	err := &pe.Error{
 		Code:         e.Code,
 		Message:      e.Message,
 		MessageChain: e.MessageChain,
 		Stack:        stackToProto(e.StackFrames),
 		Params:       e.Params,
-		Retryable:    retryable,
-		Unexpected:   unexpected,
+		Retryable:    boolValue(e.IsRetryable),
+		Unexpected:   boolValue(e.IsUnexpected),
 		MarshalCount: int32(e.MarshalCount + 1),
 	}
 	if err.Code == "" {
@@ -41,6 +31,13 @@ func Marshal(e *Error) *pe.Error {
 	return err
 }
 
+func boolValue(r *bool) *pe.BoolValue {
+	if r == nil {
+		return nil
+	}
+	return &pe.BoolValue{Value: *r}
+}
+
 // Unmarshal a protobuf error into a local error
 func Unmarshal(p *pe.Error) *Error {
 	if p == nil {

marshaling_test.go

@@ -184,14 +184,14 @@ func TestMarshal(t *testing.T) {
 		assert.Equal(t, tc.platErr.MarshalCount+1, int(protoError.MarshalCount))
 
 		if tc.platErr.IsRetryable == nil {
-			assert.False(t, protoError.Retryable.Value)
+			assert.Nil(t, protoError.Retryable)
 		} else {
 			assert.Equal(t, *tc.platErr.IsRetryable, protoError.Retryable.Value)
 			assert.Equal(t, tc.protoErr.Retryable.Value, protoError.Retryable.Value)
 		}
 
 		if tc.platErr.IsUnexpected == nil {
-			assert.False(t, protoError.Unexpected.Value)
+			assert.Nil(t, protoError.Unexpected)
 		} else {
 			assert.Equal(t, *tc.platErr.IsUnexpected, protoError.Unexpected.Value)
 			assert.Equal(t, tc.protoErr.Unexpected.Value, protoError.Unexpected.Value)
@@ -200,6 +200,18 @@ func TestMarshal(t *testing.T) {
 		if tc.platErr.MessageChain != nil {
 			assert.Equal(t, tc.platErr.MessageChain, protoError.MessageChain)
 		}
+		t.Run("unmarshalling again keeps the same values", func(t *testing.T) {
+			roundTripped := Unmarshal(protoError)
+			// The `Code` can change (e.g. to "unknown") so we cannot assert on this
+			assert.Equal(t, tc.platErr.Message, roundTripped.Message)
+			assert.Equal(t, tc.platErr.MessageChain, roundTripped.MessageChain)
+
+			assert.EqualValues(t, tc.platErr.IsRetryable, roundTripped.IsRetryable)
+			assert.EqualValues(t, tc.platErr.IsUnexpected, roundTripped.IsUnexpected)
+
+			// The marshal count is incremented by 1 on every marshal
+			assert.Equal(t, tc.platErr.MarshalCount+1, roundTripped.MarshalCount)
+		})
 	}
 }