filer: report a SQL listing that fails partway through (#12654)

* filer: report a SQL listing that fails partway through

ListDirectoryPrefixedEntries never checked rows.Err() after its rows.Next()
loop. When the result set failed partway, for example on a lost
connection, rows.Next() returned false and the listing returned the
entries read so far with a nil error, so callers saw a shorter directory
instead of an error. Check rows.Err() once the loop has run to the end,
as the scan in abstract_sql_store_scan.go already does. A listing the
callback stopped on purpose is left alone: rows.Err() would return a
canceled request context's error there.

This covers every store built on AbstractSqlStore (mysql, mysql2,
postgres, postgres2, sqlite).

Signed-off-by: anant <170962632+antcybersec@users.noreply.github.com>

* filer: wait for the canceled SQL listing to release its rows in the test

A fixed sleep let the callback return before database/sql closed the
rows, so a listing that always consulted rows.Err could still pass.
Wait, bounded, until the connection is back in the pool instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Signed-off-by: anant <170962632+antcybersec@users.noreply.github.com>
Co-authored-by: anant <170962632+antcybersec@users.noreply.github.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

weed/filer/abstract_sql/abstract_sql_store.go

@@ -428,6 +428,7 @@ func (store *AbstractSqlStore) ListDirectoryPrefixedEntries(ctx context.Context,
 	}
 	defer rows.Close()
 
+	stoppedEarly := false
 	for rows.Next() {
 		var name string
 		var data []byte
@@ -452,10 +453,20 @@ func (store *AbstractSqlStore) ListDirectoryPrefixedEntries(ctx context.Context,
 		}
 
 		if !resEachEntryFunc {
+			stoppedEarly = true
 			break
 		}
 
 	}
+	// rows.Next also returns false when the result set fails partway, for
+	// example on a lost connection; that must not read as the end of the
+	// directory. A listing the callback stopped on purpose has no such error
+	// to report (rows.Err would return a canceled context's error instead).
+	if err == nil && !stoppedEarly {
+		if rowsErr := rows.Err(); rowsErr != nil {
+			err = fmt.Errorf("list %s: %w", dirPath, rowsErr)
+		}
+	}
 
 	return lastFileName, err
 }

weed/filer/abstract_sql/abstract_sql_store_test.go

@@ -2,10 +2,13 @@ package abstract_sql
 
 import (
 	"context"
+	"errors"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/seaweedfs/seaweedfs/weed/filer"
 	"github.com/seaweedfs/seaweedfs/weed/util"
 )
 
@@ -140,3 +143,74 @@ func TestBucketHooksSkipInvalidBucketNames(t *testing.T) {
 		t.Error(err)
 	}
 }
+
+type listGen struct{ SqlGenerator }
+
+func (listGen) GetSqlListExclusive(string) string { return "SELECT name, meta" }
+func (listGen) GetSqlListInclusive(string) string { return "SELECT name, meta" }
+
+// A listing that loses its connection partway through must report the error,
+// not return the rows read so far as if the directory ended there.
+func TestListDirectoryEntriesReportsRowIterationError(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+
+	meta, err := (&filer.Entry{}).EncodeAttributesAndChunks()
+	if err != nil {
+		t.Fatal(err)
+	}
+	connLost := errors.New("connection reset by peer")
+	mock.ExpectQuery("SELECT name, meta").WillReturnRows(
+		sqlmock.NewRows([]string{"name", "meta"}).
+			AddRow("a.txt", meta).
+			AddRow("b.txt", meta).
+			RowError(1, connLost))
+
+	store := &AbstractSqlStore{SqlGenerator: listGen{}, DB: db}
+	var listed []string
+	_, err = store.ListDirectoryEntries(context.Background(), "/dir", "", true, 100, func(entry *filer.Entry) (bool, error) {
+		listed = append(listed, entry.Name())
+		return true, nil
+	})
+	if !errors.Is(err, connLost) {
+		t.Fatalf("ListDirectoryEntries error = %v, want %v (listed %v)", err, connLost, listed)
+	}
+}
+
+// A listing the callback stops on purpose is not a failure, even if the
+// request context is canceled before the rows are closed.
+func TestListDirectoryEntriesStoppedByCallbackWithCanceledContext(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+
+	meta, err := (&filer.Entry{}).EncodeAttributesAndChunks()
+	if err != nil {
+		t.Fatal(err)
+	}
+	mock.ExpectQuery("SELECT name, meta").WillReturnRows(
+		sqlmock.NewRows([]string{"name", "meta"}).AddRow("a.txt", meta).AddRow("b.txt", meta))
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	store := &AbstractSqlStore{SqlGenerator: listGen{}, DB: db}
+	_, err = store.ListDirectoryEntries(ctx, "/dir", "", true, 100, func(entry *filer.Entry) (bool, error) {
+		cancel()
+		// database/sql closes the rows on a background goroutine; once the
+		// connection is back in the pool, rows.Err reports the cancellation.
+		for deadline := time.Now().Add(5 * time.Second); db.Stats().InUse > 0; time.Sleep(time.Millisecond) {
+			if time.Now().After(deadline) {
+				t.Fatal("the canceled listing still holds its connection")
+			}
+		}
+		return false, nil
+	})
+	if err != nil {
+		t.Fatalf("ListDirectoryEntries error = %v, want nil", err)
+	}
+}