From 6532eabd75dcb7e5b2e194af9c4f253d8885f9bd Mon Sep 17 00:00:00 2001
From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com>
Date: Mon, 12 Feb 2018 12:06:48 -0500
Subject: [PATCH 1/2] Discard data more liberally on database errors.

---
 .../backends/db_fetcher/fetcher.go            | 16 ++++++---
 .../backends/db_fetcher/fetcher_test.go       | 34 ++++++++++++++++++-
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/stored_requests/backends/db_fetcher/fetcher.go b/stored_requests/backends/db_fetcher/fetcher.go
index 6e9b41862b5..e228c07692c 100644
--- a/stored_requests/backends/db_fetcher/fetcher.go
+++ b/stored_requests/backends/db_fetcher/fetcher.go
@@ -49,23 +49,31 @@ func (fetcher *dbFetcher) FetchRequests(ctx context.Context, ids []string) (map[
 		}
 		return nil, []error{err}
 	}
-	defer rows.Close()
+	defer func() {
+		if err := rows.Close(); err != nil {
+			glog.Errorf("error closing DB connection: %v", err)
+		}
+	}()
 
 	reqData := make(map[string]json.RawMessage, len(ids))
-	var errs []error = nil
 	for rows.Next() {
 		var id string
 		var thisReqData []byte
+
+		// Fixes #338?
 		if err := rows.Scan(&id, &thisReqData); err != nil {
-			errs = append(errs, err)
+			return nil, []error{err}
 		}
 
 		reqData[id] = thisReqData
 	}
+
+	// Fixes #338?
 	if rows.Err() != nil {
-		errs = append(errs, rows.Err())
+		return nil, []error{rows.Err()}
 	}
 
+	var errs []error
 	for _, id := range ids {
 		if _, ok := reqData[id]; !ok {
 			errs = append(errs, fmt.Errorf(`Stored Request with ID="%s" not found.`, id))
diff --git a/stored_requests/backends/db_fetcher/fetcher_test.go b/stored_requests/backends/db_fetcher/fetcher_test.go
index a2f906bb4d9..e3250fa7975 100644
--- a/stored_requests/backends/db_fetcher/fetcher_test.go
+++ b/stored_requests/backends/db_fetcher/fetcher_test.go
@@ -6,10 +6,12 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/DATA-DOG/go-sqlmock"
+	"log"
 	"regexp"
 	"testing"
 	"time"
+
+	"github.com/DATA-DOG/go-sqlmock"
 )
 
 func TestEmptyQuery(t *testing.T) {
@@ -165,6 +167,36 @@ func TestContextCancelled(t *testing.T) {
 	}
 }
 
+// Prevents #338?
+func TestRowErrors(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("Failed to create mock: %v", err)
+	}
+	rows := sqlmock.NewRows([]string{"id", "config"})
+	rows.AddRow("foo", []byte(`{"data":1}`))
+	rows.AddRow("bar", []byte(`{"data":2}`))
+	rows.RowError(1, errors.New("Error reading from row 1"))
+	mock.ExpectQuery(".*").WillReturnRows(rows)
+	fetcher := &dbFetcher{
+		db:         db,
+		queryMaker: successfulQueryMaker("SELECT id, config FROM my_table WHERE id IN (?)"),
+	}
+	data, errs := fetcher.FetchRequests(context.Background(), []string{"foo", "bar"})
+	for id, b := range data {
+		log.Printf("Data[%s] = %s", id, string(b))
+	}
+	if len(errs) != 1 {
+		t.Fatalf("Expected 1 error. Got %v", errs)
+	}
+	if errs[0].Error() != "Error reading from row 1" {
+		t.Errorf("Unexpected error message: %v", errs[0].Error())
+	}
+	if len(data) != 0 {
+		t.Errorf("We should not return data from the row where the error occurred.")
+	}
+}
+
 func newFetcher(rows *sqlmock.Rows, query string, args ...driver.Value) (sqlmock.Sqlmock, *dbFetcher, error) {
 	db, mock, err := sqlmock.New()
 	if err != nil {

From 7ac3c57a74593253837d2f53072eaab08a8476de Mon Sep 17 00:00:00 2001
From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com>
Date: Mon, 12 Feb 2018 12:09:06 -0500
Subject: [PATCH 2/2] Removed logs from tests.

---
 stored_requests/backends/db_fetcher/fetcher_test.go | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/stored_requests/backends/db_fetcher/fetcher_test.go b/stored_requests/backends/db_fetcher/fetcher_test.go
index e3250fa7975..df94b77b029 100644
--- a/stored_requests/backends/db_fetcher/fetcher_test.go
+++ b/stored_requests/backends/db_fetcher/fetcher_test.go
@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"log"
 	"regexp"
 	"testing"
 	"time"
@@ -183,9 +182,6 @@ func TestRowErrors(t *testing.T) {
 		queryMaker: successfulQueryMaker("SELECT id, config FROM my_table WHERE id IN (?)"),
 	}
 	data, errs := fetcher.FetchRequests(context.Background(), []string{"foo", "bar"})
-	for id, b := range data {
-		log.Printf("Data[%s] = %s", id, string(b))
-	}
 	if len(errs) != 1 {
 		t.Fatalf("Expected 1 error. Got %v", errs)
 	}