Merge pull request #1472 from marcoabreu/fix-next-swallow-errors

fix(queries): check rows.Err() after Next() loop in bind()

Author: stephenafamo

drivers/sqlboiler-mssql/driver/mssql.go

@@ -193,6 +193,10 @@ func (m *MSSQLDriver) TableNames(schema string, whitelist, blacklist []string) (
 		names = append(names, name)
 	}
 
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
 	return names, nil
 }
 
@@ -240,6 +244,10 @@ func (m *MSSQLDriver) ViewNames(schema string, whitelist, blacklist []string) ([
 		names = append(names, name)
 	}
 
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
 	return names, nil
 }
 
@@ -359,6 +367,10 @@ func (m *MSSQLDriver) Columns(schema, tableName string, whitelist, blacklist []s
 		columns = append(columns, column)
 	}
 
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
 	return columns, nil
 }
 

drivers/sqlboiler-mssql/driver/mssql_test.go

@@ -26,11 +26,13 @@ import (
 	"bytes"
 	"encoding/json"
 	"flag"
+	"fmt"
 	"os"
 	"os/exec"
 	"regexp"
 	"testing"
 
+	"github.com/DATA-DOG/go-sqlmock"
 	"github.com/aarondl/sqlboiler/v4/drivers"
 )
 
@@ -139,3 +141,108 @@ func TestDriver(t *testing.T) {
 		}
 	})
 }
+
+// TestTableNames_RowsErr verifies that TableNames checks rows.Err() after
+// iterating and propagates any error encountered during row iteration back
+// to the caller rather than silently returning partial results.
+func TestTableNames_RowsErr(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+
+	simulatedErr := fmt.Errorf("rows iteration error")
+
+	rows := sqlmock.NewRows([]string{"table_name"}).
+		AddRow("table1").
+		RowError(0, simulatedErr)
+
+	mock.ExpectQuery(`SELECT table_name`).
+		WithArgs("dbo").
+		WillReturnRows(rows)
+
+	driver := &MSSQLDriver{conn: db}
+	_, err = driver.TableNames("dbo", nil, nil)
+	if err == nil {
+		t.Fatal("expected error from rows.Err(), got nil")
+	}
+	if err.Error() != simulatedErr.Error() {
+		t.Errorf("expected error %q, got %q", simulatedErr, err)
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unfulfilled expectations: %s", err)
+	}
+}
+
+// TestViewNames_RowsErr verifies that ViewNames checks rows.Err() after
+// iterating and propagates any error encountered during row iteration back
+// to the caller rather than silently returning partial results.
+func TestViewNames_RowsErr(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+
+	simulatedErr := fmt.Errorf("rows iteration error")
+
+	rows := sqlmock.NewRows([]string{"table_name"}).
+		AddRow("view1").
+		RowError(0, simulatedErr)
+
+	mock.ExpectQuery(`select table_name`).
+		WithArgs("dbo").
+		WillReturnRows(rows)
+
+	driver := &MSSQLDriver{conn: db}
+	_, err = driver.ViewNames("dbo", nil, nil)
+	if err == nil {
+		t.Fatal("expected error from rows.Err(), got nil")
+	}
+	if err.Error() != simulatedErr.Error() {
+		t.Errorf("expected error %q, got %q", simulatedErr, err)
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unfulfilled expectations: %s", err)
+	}
+}
+
+// TestColumns_RowsErr verifies that Columns checks rows.Err() after iterating
+// and propagates any error encountered during row iteration back to the caller
+// rather than silently returning partial results.
+func TestColumns_RowsErr(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+
+	simulatedErr := fmt.Errorf("rows iteration error")
+
+	rows := sqlmock.NewRows([]string{
+		"column_name", "full_type", "data_type", "column_default",
+		"is_nullable", "is_unique", "is_identity", "is_computed",
+	}).
+		AddRow("id", "int", "int", nil, false, true, true, false).
+		RowError(0, simulatedErr)
+
+	mock.ExpectQuery(`SELECT column_name`).
+		WithArgs("dbo", "test_table").
+		WillReturnRows(rows)
+
+	driver := &MSSQLDriver{conn: db}
+	_, err = driver.Columns("dbo", "test_table", nil, nil)
+	if err == nil {
+		t.Fatal("expected error from rows.Err(), got nil")
+	}
+	if err.Error() != simulatedErr.Error() {
+		t.Errorf("expected error %q, got %q", simulatedErr, err)
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unfulfilled expectations: %s", err)
+	}
+}

drivers/sqlboiler-mysql/driver/mysql.go

@@ -222,6 +222,9 @@ func (m *MySQLDriver) TableNames(schema string, whitelist, blacklist []string) (
 		}
 		names = append(names, name)
 	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
 
 	return names, nil
 }
@@ -269,6 +272,9 @@ func (m *MySQLDriver) ViewNames(schema string, whitelist, blacklist []string) ([
 
 		names = append(names, name)
 	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
 
 	return names, nil
 }
@@ -381,6 +387,9 @@ func (m *MySQLDriver) Columns(schema, tableName string, whitelist, blacklist []s
 
 		columns = append(columns, column)
 	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
 
 	return columns, nil
 }

drivers/sqlboiler-mysql/driver/mysql_test.go

@@ -18,6 +18,7 @@ import (
 	"os/exec"
 	"testing"
 
+	"github.com/DATA-DOG/go-sqlmock"
 	"github.com/stretchr/testify/require"
 	"github.com/aarondl/sqlboiler/v4/drivers"
 )
@@ -142,3 +143,84 @@ func TestDriver(t *testing.T) {
 		require.False(t, found, "blacklisted column 'string_three' should not be present in table 'magic'")
 	})
 }
+
+// TestTableNames_RowsErr verifies that TableNames propagates errors returned
+// by rows.Err() after the row-iteration loop. A RowError on the first row
+// simulates a mid-iteration failure (e.g. a dropped connection) and the test
+// asserts that the caller receives the underlying error instead of a nil.
+func TestTableNames_RowsErr(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	require.NoError(t, err)
+	defer db.Close()
+
+	simulatedErr := fmt.Errorf("simulated row iteration error")
+
+	rows := sqlmock.NewRows([]string{"table_name"}).
+		AddRow("table1").
+		RowError(0, simulatedErr)
+
+	mock.ExpectQuery(`select table_name from information_schema\.tables`).
+		WithArgs("testschema").
+		WillReturnRows(rows)
+
+	m := &MySQLDriver{conn: db}
+	_, err = m.TableNames("testschema", nil, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "simulated row iteration error")
+	require.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestViewNames_RowsErr verifies that ViewNames propagates errors returned
+// by rows.Err() after the row-iteration loop. A RowError on the first row
+// simulates a mid-iteration failure and the test asserts that the caller
+// receives the underlying error instead of a nil.
+func TestViewNames_RowsErr(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	require.NoError(t, err)
+	defer db.Close()
+
+	simulatedErr := fmt.Errorf("simulated row iteration error")
+
+	rows := sqlmock.NewRows([]string{"table_name"}).
+		AddRow("view1").
+		RowError(0, simulatedErr)
+
+	mock.ExpectQuery(`select table_name from information_schema\.views`).
+		WithArgs("testschema").
+		WillReturnRows(rows)
+
+	m := &MySQLDriver{conn: db}
+	_, err = m.ViewNames("testschema", nil, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "simulated row iteration error")
+	require.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestColumns_RowsErr verifies that Columns propagates errors returned by
+// rows.Err() after the row-iteration loop. A RowError on the first row
+// simulates a mid-iteration failure and the test asserts that the caller
+// receives the underlying error instead of a nil.
+func TestColumns_RowsErr(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	require.NoError(t, err)
+	defer db.Close()
+
+	simulatedErr := fmt.Errorf("simulated row iteration error")
+
+	rows := sqlmock.NewRows([]string{
+		"column_name", "column_type", "column_comment", "data_type",
+		"column_default", "is_nullable", "is_generated", "is_unique",
+	}).
+		AddRow("id", "int", "", "int", nil, false, false, true).
+		RowError(0, simulatedErr)
+
+	mock.ExpectQuery(`select\s+c\.column_name`).
+		WithArgs("test_table", "test_table", "testschema", "testschema", "testschema", "testschema", "test_table", "test_table", "testschema").
+		WillReturnRows(rows)
+
+	m := &MySQLDriver{conn: db}
+	_, err = m.Columns("testschema", "test_table", nil, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "simulated row iteration error")
+	require.NoError(t, mock.ExpectationsWereMet())
+}

drivers/sqlboiler-psql/driver/psql.go

@@ -223,6 +223,10 @@ func (p *PostgresDriver) TableNames(schema string, whitelist, blacklist []string
 		names = append(names, name)
 	}
 
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
 	return names, nil
 }
 
@@ -281,6 +285,10 @@ func (p *PostgresDriver) ViewNames(schema string, whitelist, blacklist []string)
 		names = append(names, name)
 	}
 
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
 	return names, nil
 }
 
@@ -387,6 +395,11 @@ select * from results;
 		}
 		p.uniqueColumns.Store(c, struct{}{})
 	}
+
+	if err := rows.Err(); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -661,6 +674,10 @@ func (p *PostgresDriver) Columns(schema, tableName string, whitelist, blacklist
 		columns = append(columns, column)
 	}
 
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
 	return columns, nil
 }