Partial unpack (#300)

* Return partial message when unpack errors * Stop parsing at error, return partial message * Remove wrappErrorUnpack * Revert improvement to Unmarshal(), add comment to test * Rename Field to FieldID * Put error wrapping function back --------- Co-authored-by: eileen <eileen@thoughtmachine.net>
moov-io · Nov 30, 2023 · 231e13d · 231e13d
1 parent 8f4cb18
commit 231e13d
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 10 deletions.
diff --git a/errors.go b/errors.go
@@ -4,6 +4,7 @@ package iso8583
 // connection failed to unpack message
 type UnpackError struct {
 	Err        error
+	FieldID    string
 	RawMessage []byte
 }
 

diff --git a/message.go b/message.go
@@ -230,16 +230,17 @@ func (m *Message) Unpack(src []byte) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	return m.wrappErrorUnpack(src)
+	return m.wrapErrorUnpack(src)
 }
 
-// wrappErrorUnpack calls the core unpacking logic and wraps any
+// wrapErrorUnpack calls the core unpacking logic and wraps any
 // errors in a *UnpackError. It assumes that the mutex is already
 // locked by the caller.
-func (m *Message) wrappErrorUnpack(src []byte) error {
-	if err := m.unpack(src); err != nil {
+func (m *Message) wrapErrorUnpack(src []byte) error {
+	if fieldID, err := m.unpack(src); err != nil {
 		return &UnpackError{
 			Err:        err,
+			FieldID:    fieldID,
 			RawMessage: src,
 		}
 	}
@@ -249,7 +250,7 @@ func (m *Message) wrappErrorUnpack(src []byte) error {
 // unpack contains the core logic for unpacking the message. This method does
 // not handle locking or error wrapping and should typically be used internally
 // after ensuring concurrency safety.
-func (m *Message) unpack(src []byte) error {
+func (m *Message) unpack(src []byte) (string, error) {
 	var off int
 
 	// reset fields that were set
@@ -260,7 +261,7 @@ func (m *Message) unpack(src []byte) error {
 
 	read, err := m.fields[mtiIdx].Unpack(src)
 	if err != nil {
-		return fmt.Errorf("failed to unpack MTI: %w", err)
+		return strconv.Itoa(mtiIdx), fmt.Errorf("failed to unpack MTI: %w", err)
 	}
 
 	m.fieldsMap[mtiIdx] = struct{}{}
@@ -270,7 +271,7 @@ func (m *Message) unpack(src []byte) error {
 	// unpack Bitmap
 	read, err = m.fields[bitmapIdx].Unpack(src[off:])
 	if err != nil {
-		return fmt.Errorf("failed to unpack bitmap: %w", err)
+		return strconv.Itoa(bitmapIdx), fmt.Errorf("failed to unpack bitmap: %w", err)
 	}
 
 	off += read
@@ -284,12 +285,12 @@ func (m *Message) unpack(src []byte) error {
 		if m.bitmap().IsSet(i) {
 			fl, ok := m.fields[i]
 			if !ok {
-				return fmt.Errorf("failed to unpack field %d: no specification found", i)
+				return strconv.Itoa(i), fmt.Errorf("failed to unpack field %d: no specification found", i)
 			}
 
 			read, err = fl.Unpack(src[off:])
 			if err != nil {
-				return fmt.Errorf("failed to unpack field %d (%s): %w", i, fl.Spec().Description, err)
+				return strconv.Itoa(i), fmt.Errorf("failed to unpack field %d (%s): %w", i, fl.Spec().Description, err)
 			}
 
 			m.fieldsMap[i] = struct{}{}
@@ -298,7 +299,7 @@ func (m *Message) unpack(src []byte) error {
 		}
 	}
 
-	return nil
+	return "", nil
 }
 
 func (m *Message) MarshalJSON() ([]byte, error) {

diff --git a/message_test.go b/message_test.go
@@ -751,6 +751,62 @@ func TestPackUnpack(t *testing.T) {
 		require.Equal(t, rawMsg, unpackError.RawMessage)
 	})
 
+	t.Run("Unpack data field error on field returns partial message", func(t *testing.T) {
+		message := NewMessage(spec)
+
+		// One byte has been removed from field 120 which will make it fail the length check
+		rawMsg := []byte{0x30, 0x31, 0x30, 0x30, 0xf2, 0x3c, 0x24, 0x81, 0x28, 0xe0, 0x9a, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x31, 0x36, 0x34, 0x32, 0x37, 0x36, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x37, 0x37, 0x37, 0x30, 0x30, 0x30, 0x37, 0x30, 0x31, 0x31, 0x31, 0x31, 0x38, 0x34, 0x34, 0x30, 0x30, 0x30, 0x31, 0x32, 0x33, 0x31, 0x33, 0x31, 0x38, 0x34, 0x34, 0x30, 0x37, 0x30, 0x31, 0x31, 0x39, 0x30, 0x32, 0x6, 0x43, 0x39, 0x30, 0x31, 0x30, 0x32, 0x30, 0x36, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x33, 0x37, 0x34, 0x32, 0x37, 0x36, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x3d, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x39, 0x38, 0x37, 0x36, 0x35, 0x34, 0x33, 0x32, 0x31, 0x30, 0x30, 0x31, 0x30, 0x30, 0x30, 0x30, 0x30, 0x33, 0x32, 0x31, 0x31, 0x32, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x33, 0x34, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x54, 0x65, 0x73, 0x74, 0x20, 0x74, 0x65, 0x78, 0x74, 0x64, 0x30, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x31, 0x32, 0x33, 0x34, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x32, 0x33, 0x9a, 0x6, 0x32, 0x31, 0x30, 0x37, 0x32, 0x30, 0x9f, 0x2, 0xc, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x35, 0x30, 0x31, 0x30, 0x31, 0x37, 0x41, 0x6e, 0x6f, 0x74, 0x68, 0x65, 0x72, 0x20, 0x74, 0x65, 0x73, 0x74, 0x20, 0x74, 0x65, 0x78}
+
+		err := message.Unpack([]byte(rawMsg))
+
+		require.Error(t, err)
+		var unpackError *UnpackError
+		require.ErrorAs(t, err, &unpackError)
+		assert.Equal(t, unpackError.FieldID, "120")
+
+		s, err := message.GetString(2)
+		require.NoError(t, err)
+		require.Equal(t, "4276555555555555", s)
+
+		s, err = message.GetString(3)
+		require.NoError(t, err)
+		require.Equal(t, "000000", s)
+
+		s, err = message.GetString(4)
+		require.NoError(t, err)
+		require.Equal(t, "77700", s)
+
+		data := &TestISOData{}
+		require.NoError(t, message.Unmarshal(data))
+
+		assert.Equal(t, "4276555555555555", data.F2.Value())
+		assert.Equal(t, "00", data.F3.F1.Value())
+		assert.Equal(t, "00", data.F3.F2.Value())
+		assert.Equal(t, "00", data.F3.F3.Value())
+		assert.Equal(t, int64(77700), data.F4.Value())
+		assert.Equal(t, int64(701111844), data.F7.Value())
+		assert.Equal(t, int64(123), data.F11.Value())
+		assert.Equal(t, int64(131844), data.F12.Value())
+		assert.Equal(t, int64(701), data.F13.Value())
+		assert.Equal(t, int64(1902), data.F14.Value())
+		assert.Equal(t, int64(643), data.F19.Value())
+		assert.Equal(t, int64(901), data.F22.Value())
+		assert.Equal(t, int64(2), data.F25.Value())
+		assert.Equal(t, int64(123456), data.F32.Value())
+		assert.Equal(t, "4276555555555555=12345678901234567890", data.F35.Value())
+		assert.Equal(t, "987654321001", data.F37.Value())
+		assert.Equal(t, "00000321", data.F41.Value())
+		assert.Equal(t, "120000000000034", data.F42.Value())
+		assert.Equal(t, "Test text", data.F43.Value())
+		assert.Equal(t, int64(643), data.F49.Value())
+		assert.Nil(t, data.F50)
+		assert.Equal(t, string([]byte{1, 2, 3, 4, 5, 6, 7, 8}), data.F52.Value())
+		assert.Equal(t, int64(1234000000000000), data.F53.Value())
+		assert.Equal(t, "210720", data.F55.F9A.Value())
+		assert.Equal(t, "000000000501", data.F55.F9F02.Value())
+		assert.Empty(t, data.F120)
+	})
+
 	// this test should check that BCD fields are packed and
 	// unpacked correctly it's a confirmation that issue
 	// https://github.com/moov-io/iso8583/issues/220 is fixed