Skip to content

Commit

Permalink
Return subfields in unpack error (#313)
Browse files Browse the repository at this point in the history
* Return subfield IDs upon unpack error

* Tidy test

* Comments

* Make linter happy

* Remove FieldIDs
  • Loading branch information
meparle committed Jul 15, 2024
1 parent edc9020 commit dfac6bd
Show file tree
Hide file tree
Showing 6 changed files with 314 additions and 54 deletions.
29 changes: 0 additions & 29 deletions errors.go

This file was deleted.

50 changes: 50 additions & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package errors

import (
"errors"
)

// UnpackError returns an error with the possibility to access the RawMessage when
// the connection failed to unpack the message
type UnpackError struct {
Err error
FieldID string
RawMessage []byte
}

func (e *UnpackError) Error() string {
return e.Err.Error()
}

func (e *UnpackError) Unwrap() error {
return e.Err
}

// FieldIDs returns the list of field and subfield IDs (if any) that errored from outermost inwards
func (e *UnpackError) FieldIDs() []string {
fieldIDs := []string{e.FieldID}
err := e.Err
var unpackError *UnpackError
for {
if errors.As(err, &unpackError) {
fieldIDs = append(fieldIDs, unpackError.FieldID)
err = unpackError.Unwrap()
} else {
break
}
}

return fieldIDs
}

type PackError struct {
Err error
}

func (e *PackError) Error() string {
return e.Err.Error()
}

func (e *PackError) Unwrap() error {
return e.Err
}
54 changes: 35 additions & 19 deletions field/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"sync"

"github.com/moov-io/iso8583/encoding"
iso8583errors "github.com/moov-io/iso8583/errors"
"github.com/moov-io/iso8583/prefix"
"github.com/moov-io/iso8583/sort"
"github.com/moov-io/iso8583/utils"
Expand Down Expand Up @@ -314,7 +315,7 @@ func (f *Composite) Unpack(data []byte) (int, error) {
// data is stripped of the prefix before it is provided to unpack().
// Therefore, it is unaware of when to stop parsing unless we bound the
// length of the slice by the data length.
read, err := f.unpack(data[offset:offset+dataLen], isVariableLength)
read, err := f.wrapErrorUnpack(data[offset:offset+dataLen], isVariableLength)
if err != nil {
return 0, err
}
Expand All @@ -333,7 +334,7 @@ func (f *Composite) SetBytes(data []byte) error {
f.mu.Lock()
defer f.mu.Unlock()

_, err := f.unpack(data, false)
_, err := f.wrapErrorUnpack(data, false)
return err
}

Expand Down Expand Up @@ -517,7 +518,22 @@ func (f *Composite) packByTag() ([]byte, error) {
return packed, nil
}

func (f *Composite) unpack(data []byte, isVariableLength bool) (int, error) {
// 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 (f *Composite) wrapErrorUnpack(src []byte, isVariableLength bool) (int, error) {
offset, tagID, err := f.unpack(src, isVariableLength)
if err != nil {
return offset, &iso8583errors.UnpackError{
Err: err,
FieldID: tagID,
RawMessage: src,
}
}
return offset, nil
}

func (f *Composite) unpack(data []byte, isVariableLength bool) (int, string, error) {
if f.bitmap() != nil {
return f.unpackSubfieldsByBitmap(data)
}
Expand All @@ -527,7 +543,7 @@ func (f *Composite) unpack(data []byte, isVariableLength bool) (int, error) {
return f.unpackSubfields(data, isVariableLength)
}

func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, error) {
func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, string, error) {
offset := 0
for _, tag := range f.orderedSpecFieldTags {
field, ok := f.subfields[tag]
Expand All @@ -537,7 +553,7 @@ func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, er

read, err := field.Unpack(data[offset:])
if err != nil {
return 0, fmt.Errorf("failed to unpack subfield %v: %w", tag, err)
return 0, tag, fmt.Errorf("failed to unpack subfield %v: %w", tag, err)
}

f.setSubfields[tag] = struct{}{}
Expand All @@ -549,10 +565,10 @@ func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, er
}
}

return offset, nil
return offset, "", nil
}

func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) {
func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, string, error) {
var off int

// Reset fields that were set.
Expand All @@ -562,7 +578,7 @@ func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) {

read, err := f.bitmap().Unpack(data[off:])
if err != nil {
return 0, fmt.Errorf("failed to unpack bitmap: %w", err)
return 0, "", fmt.Errorf("failed to unpack bitmap: %w", err)
}

off += read
Expand All @@ -573,12 +589,12 @@ func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) {

fl, ok := f.subfields[iStr]
if !ok {
return 0, fmt.Errorf("failed to unpack subfield %s: no specification found", iStr)
return 0, iStr, fmt.Errorf("failed to unpack subfield %s: no specification found", iStr)
}

read, err = fl.Unpack(data[off:])
if err != nil {
return 0, fmt.Errorf("failed to unpack subfield %s (%s): %w", iStr, fl.Spec().Description, err)
return 0, iStr, fmt.Errorf("failed to unpack subfield %s (%s): %w", iStr, fl.Spec().Description, err)
}

f.setSubfields[iStr] = struct{}{}
Expand All @@ -587,7 +603,7 @@ func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) {
}
}

return off, nil
return off, "", nil
}

const (
Expand All @@ -598,12 +614,12 @@ const (
maxLenOfUnknownTag = math.MaxInt
)

func (f *Composite) unpackSubfieldsByTag(data []byte) (int, error) {
func (f *Composite) unpackSubfieldsByTag(data []byte) (int, string, error) {
offset := 0
for offset < len(data) {
tagBytes, read, err := f.spec.Tag.Enc.Decode(data[offset:], f.spec.Tag.Length)
if err != nil {
return 0, fmt.Errorf("failed to unpack subfield Tag: %w", err)
return 0, "", fmt.Errorf("failed to unpack subfield Tag: %w", err)
}
offset += read

Expand All @@ -625,15 +641,15 @@ func (f *Composite) unpackSubfieldsByTag(data []byte) (int, error) {
maxLen = maxLenOfUnknownTag
}

fieldLength, readed, err := pref.DecodeLength(maxLen, data[offset:])
fieldLength, read, err := pref.DecodeLength(maxLen, data[offset:])
if err != nil {
return 0, err
return 0, "", err
}
offset += fieldLength + readed
offset += fieldLength + read
continue
}

return 0, fmt.Errorf("failed to unpack subfield %v: field not defined in Spec", tag)
return 0, tag, fmt.Errorf("failed to unpack subfield %v: field not defined in Spec", tag)
}

field, ok := f.subfields[tag]
Expand All @@ -643,14 +659,14 @@ func (f *Composite) unpackSubfieldsByTag(data []byte) (int, error) {

read, err = field.Unpack(data[offset:])
if err != nil {
return 0, fmt.Errorf("failed to unpack subfield %v: %w", tag, err)
return 0, tag, fmt.Errorf("failed to unpack subfield %v: %w", tag, err)
}

f.setSubfields[tag] = struct{}{}

offset += read
}
return offset, nil
return offset, "", nil
}

func (f *Composite) skipUnknownTLVTags() bool {
Expand Down
6 changes: 6 additions & 0 deletions field/composite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"testing"

"github.com/moov-io/iso8583/encoding"
iso8583errors "github.com/moov-io/iso8583/errors"
"github.com/moov-io/iso8583/padding"
"github.com/moov-io/iso8583/prefix"
"github.com/moov-io/iso8583/sort"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -799,6 +801,10 @@ func TestCompositePacking(t *testing.T) {
read, err := composite.Unpack([]byte("ABCDEF"))
require.Equal(t, 0, read)
require.Error(t, err)
var unpackError *iso8583errors.UnpackError
require.ErrorAs(t, err, &unpackError)
assert.Equal(t, "3", unpackError.FieldID)
assert.Equal(t, []string{"3"}, unpackError.FieldIDs())
require.EqualError(t, err, "failed to unpack subfield 3: failed to set bytes: failed to convert into number")
require.ErrorIs(t, err, strconv.ErrSyntax)
})
Expand Down
5 changes: 3 additions & 2 deletions message.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strconv"
"sync"

iso8583errors "github.com/moov-io/iso8583/errors"
"github.com/moov-io/iso8583/field"
"github.com/moov-io/iso8583/utils"
)
Expand Down Expand Up @@ -175,7 +176,7 @@ func (m *Message) Pack() ([]byte, error) {
func (m *Message) wrapErrorPack() ([]byte, error) {
data, err := m.pack()
if err != nil {
return nil, &PackError{Err: err}
return nil, &iso8583errors.PackError{Err: err}
}

return data, nil
Expand Down Expand Up @@ -238,7 +239,7 @@ func (m *Message) Unpack(src []byte) error {
// locked by the caller.
func (m *Message) wrapErrorUnpack(src []byte) error {
if fieldID, err := m.unpack(src); err != nil {
return &UnpackError{
return &iso8583errors.UnpackError{
Err: err,
FieldID: fieldID,
RawMessage: src,
Expand Down
Loading

0 comments on commit dfac6bd

Please sign in to comment.