Merge pull request #1203 from adamdecaf/issue1202

fix: accumulate batches without BatchControl records

fileErrors.go

@@ -37,8 +37,8 @@ var (
 	ErrFileAddendaOutsideEntry = errors.New("addenda outside of entry")
 	// ErrFileBatchControlOutsideBatch is the error given if a batch control record is outside of a batch
 	ErrFileBatchControlOutsideBatch = errors.New("batch control outside of batch")
-	// ErrFileBatchHeaderInsideBatch is the error given if a batch header record is inside of a batch
-	ErrFileBatchHeaderInsideBatch = errors.New("batch header inside of batch")
+	// ErrFileConsecutiveBatchHeaders is the error given when multiple batch header records occur in sequence
+	ErrFileConsecutiveBatchHeaders = errors.New("consecutive Batch Headers in file")
 	// ErrFileADVOnly is the error given if an ADV only file has a non-ADV batch
 	ErrFileADVOnly = errors.New("file can only have ADV Batches")
 	// ErrFileIATSEC is the error given if an IAT batch uses the normal NewBatch

reader.go

@@ -270,6 +270,18 @@ func (r *Reader) parseLine() error {
 			return err
 		}
 	case batchHeaderPos:
+		// We can sometimes run into files that have (BH, ED, ED...) records
+		// without BatchControls. We need to still accumulate Batches.
+		if r.currentBatch != nil {
+			if len(r.currentBatch.GetEntries()) == 0 {
+				return ErrFileConsecutiveBatchHeaders
+			}
+			// Accumulate currentBatch before parsing another Batch
+			batch := r.currentBatch
+			r.currentBatch = nil
+			batch.SetValidation(r.File.validateOpts)
+			r.File.AddBatch(batch)
+		}
 		if err := r.parseBH(); err != nil {
 			return err
 		}
@@ -380,10 +392,6 @@ func (r *Reader) parseFileHeader() error {
 // parseBatchHeader takes the input record string and parses the FileHeaderRecord values
 func (r *Reader) parseBatchHeader() error {
 	r.recordName = "BatchHeader"
-	if r.currentBatch != nil {
-		// batch header inside of current batch
-		return ErrFileBatchHeaderInsideBatch
-	}
 
 	// Ensure we have a valid batch header before building a batch.
 	bh := NewBatchHeader()
@@ -595,10 +603,6 @@ func (r *Reader) parseFileControl() error {
 // parseIATBatchHeader takes the input record string and parses the FileHeaderRecord values
 func (r *Reader) parseIATBatchHeader() error {
 	r.recordName = "BatchHeader"
-	if r.IATCurrentBatch.Header != nil {
-		// batch header inside of current batch
-		return ErrFileBatchHeaderInsideBatch
-	}
 
 	// Ensure we have a valid IAT BatchHeader before building a batch.
 	bh := NewIATBatchHeader()

reader_test.go

@@ -71,17 +71,12 @@ func TestReadPartial(t *testing.T) {
 	if err != nil {
 		t.Log(err) // we expect errors -- display them
 	}
-	if len(file.Batches) != 1 {
-		t.Fatalf("unexpected batches: %#v", file.Batches)
-	}
-	entries := file.Batches[0].GetEntries()
-	if len(entries) != 3 {
-		t.Error("unexpected entry details")
-		for i := range entries {
-			t.Errorf("  %#v", entries[i])
-		}
-		t.Fatal("")
-	}
+	require.Len(t, file.Batches, 2)
+
+	b1Entries := file.Batches[0].GetEntries()
+	require.Len(t, b1Entries, 1)
+	require.Nil(t, b1Entries[0].Addenda98)
+	require.NotNil(t, b1Entries[0].Addenda99)
 
 	// batch
 	bh := file.Batches[0].GetHeader()
@@ -99,7 +94,7 @@ func TestReadPartial(t *testing.T) {
 	}
 
 	// entries
-	entry := entries[0]
+	entry := b1Entries[0]
 	if entry.TransactionCode != CheckingReturnNOCDebit {
 		t.Errorf("TransactionCode=%d", entry.TransactionCode)
 	}
@@ -114,7 +109,15 @@ func TestReadPartial(t *testing.T) {
 		t.Errorf("nil Addenda99")
 	}
 
-	entry = entries[1]
+	b2Entries := file.Batches[1].GetEntries()
+	require.Len(t, b2Entries, 2)
+
+	for i := range b2Entries {
+		require.Nil(t, b2Entries[i].Addenda98)
+		require.NotNil(t, b2Entries[i].Addenda99)
+	}
+
+	entry = b2Entries[0]
 	if entry.TransactionCode != CheckingReturnNOCCredit {
 		t.Errorf("TransactionCode=%d", entry.TransactionCode)
 	}
@@ -583,7 +586,7 @@ func testFileBatchHeaderDuplicate(t testing.TB) {
 	r.addCurrentBatch(NewBatchPPD(bh))
 	// read should fail because it is parsing a second batch header and there can only be one.
 	_, err := r.Read()
-	if !base.Has(err, ErrFileBatchHeaderInsideBatch) {
+	if !base.Has(err, ErrFileConsecutiveBatchHeaders) {
 		t.Errorf("%T: %s", err, err)
 	}
 }
@@ -1644,7 +1647,10 @@ func testACHFileIATBH(t testing.TB) {
 	r := NewReader(f)
 	_, err = r.Read()
 
-	if !base.Has(err, ErrFileBatchHeaderInsideBatch) {
+	if !base.Has(err, ErrFileAddendaOutsideEntry) {
+		t.Errorf("%T: %s", err, err)
+	}
+	if !base.Has(err, ErrFileBatchControlOutsideBatch) {
 		t.Errorf("%T: %s", err, err)
 	}
 }

test/issues/issue1202_test.go

@@ -0,0 +1,34 @@
+package issues
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/moov-io/ach"
+	"github.com/moov-io/ach/cmd/achcli/describe"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIssue1202(t *testing.T) {
+	file, err := ach.ReadFile(filepath.Join("..", "..", "test", "testdata", "return-no-batch-controls.ach"))
+	require.ErrorContains(t, err, ach.ErrFileHeader.Error())
+
+	if testing.Verbose() {
+		describe.File(os.Stdout, file, nil)
+	}
+
+	require.Len(t, file.Batches, 2)
+	require.Len(t, file.NotificationOfChange, 1)
+	require.Len(t, file.ReturnEntries, 1)
+
+	b1Entries := file.Batches[0].GetEntries()
+	require.Len(t, b1Entries, 1)
+	require.Nil(t, b1Entries[0].Addenda98)
+	require.NotNil(t, b1Entries[0].Addenda99)
+
+	b2Entries := file.Batches[1].GetEntries()
+	require.Len(t, b2Entries, 1)
+	require.NotNil(t, b2Entries[0].Addenda98)
+	require.Nil(t, b2Entries[0].Addenda99)
+}

test/testdata/return-no-batch-controls.ach

@@ -0,0 +1,6 @@
+5200CoinLion                            123456789 WEBTRANSFER        000101   1091000010000001
+626091400606123456789        0000012354MjMxNDAwMjAtOGQPaul Jones            S 1091000017611242
+799R01091400600000001      09100001                                            091000017611242
+5220Your Company, in                    121042882 CORVendor Pay      000000   1121042880000001
+621231380104744-5678-99      0000000000location #23   Best Co. #23          S 1121042880000001
+798C01121042880000001      121042881918171614                                  091012980000088