Skip to content

Commit

Permalink
Discard data more liberally on database errors. (#339)
Browse files Browse the repository at this point in the history
* Discard data more liberally on database errors.

* Removed logs from tests.
  • Loading branch information
dbemiller authored Feb 20, 2018
1 parent ce04789 commit 9b68f29
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
16 changes: 12 additions & 4 deletions stored_requests/backends/db_fetcher/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
30 changes: 29 additions & 1 deletion stored_requests/backends/db_fetcher/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/DATA-DOG/go-sqlmock"
"regexp"
"testing"
"time"

"github.com/DATA-DOG/go-sqlmock"
)

func TestEmptyQuery(t *testing.T) {
Expand Down Expand Up @@ -165,6 +166,33 @@ 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"})
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 {
Expand Down

0 comments on commit 9b68f29

Please sign in to comment.