Enforced Google's specification regarding back-off mode

api.go

@@ -17,17 +17,25 @@ package safebrowsing
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io/ioutil"
+	"math/rand"
 	"net/http"
 	"net/url"
 	"strings"
+	"time"
 
 	pb "github.com/google/safebrowsing/internal/safebrowsing_proto"
 
 	"github.com/golang/protobuf/proto"
 )
 
+const (
+	maxRetryDelay  = 24 * time.Hour
+	baseRetryDelay = 15 * time.Minute
+)
+
 const (
 	findHashPath    = "/v4/fullHashes:find"
 	fetchUpdatePath = "/v4/threatListUpdates:fetch"
@@ -41,8 +49,10 @@ type api interface {
 
 // netAPI is an api object that talks to the server over HTTP.
 type netAPI struct {
-	client *http.Client
-	url    *url.URL
+	client          *http.Client
+	url             *url.URL
+	backOffMode     bool
+	updateAPIErrors uint
 }
 
 // newNetAPI creates a new netAPI object pointed at the provided root URL.
@@ -72,13 +82,17 @@ func newNetAPI(root string, key string, proxy string) (*netAPI, error) {
 	q.Set("key", key)
 	q.Set("alt", "proto")
 	u.RawQuery = q.Encode()
-	return &netAPI{url: u, client: httpClient}, nil
+	return &netAPI{url: u, client: httpClient, backOffMode: false}, nil
 }
 
 // doRequests performs a POST to requestPath. It uses the marshaled form of req
 // as the request body payload, and automatically unmarshals the response body
 // payload as resp.
 func (a *netAPI) doRequest(ctx context.Context, requestPath string, req proto.Message, resp proto.Message) error {
+	if a.backOffMode {
+		return errors.New("API in back-off mode, refusing to execute")
+	}
+
 	p, err := proto.Marshal(req)
 	if err != nil {
 		return err
@@ -95,8 +109,25 @@ func (a *netAPI) doRequest(ctx context.Context, requestPath string, req proto.Me
 	}
 	defer httpResp.Body.Close()
 	if httpResp.StatusCode != 200 {
+		// Non 200 response, we shall enter back-off mode
+		a.backOffMode = true
+		go func(a *netAPI) {
+			// Backoff strategy: MIN((2**N-1 * 15 minutes) * (RAND + 1), 24 hours)
+			n := 1 << a.updateAPIErrors
+			delay := time.Duration(float64(n) * (rand.Float64() + 1) * float64(baseRetryDelay))
+			if delay > maxRetryDelay {
+				delay = maxRetryDelay
+			}
+			a.updateAPIErrors++
+			time.Sleep(delay)
+			a.backOffMode = false
+		}(a)
+
 		return fmt.Errorf("safebrowsing: unexpected server response code: %d", httpResp.StatusCode)
 	}
+
+	a.updateAPIErrors = 0
+
 	body, err := ioutil.ReadAll(httpResp.Body)
 	if err != nil {
 		return err

database.go

@@ -33,8 +33,6 @@ import (
 // actually take. We add this time to the update period time to give some
 // leeway before declaring the database as stale.
 const (
-	maxRetryDelay  = 24 * time.Hour
-	baseRetryDelay = 15 * time.Minute
 	jitter         = 30 * time.Second
 )
 
@@ -72,7 +70,6 @@ type database struct {
 	err             error         // Last error encountered
 	readyCh         chan struct{} // Used for waiting until not in an error state.
 	last            time.Time     // Last time the threat list were synced
-	updateAPIErrors uint          // Number of times we attempted to contact the api and failed
 
 	log *log.Logger
 }
@@ -221,18 +218,12 @@ func (db *database) Update(ctx context.Context, api api) (time.Duration, bool) {
 	last := db.config.now()
 	resp, err := api.ListUpdate(ctx, req)
 	if err != nil {
-		db.log.Printf("ListUpdate failure (%d): %v", db.updateAPIErrors+1, err)
+		db.log.Printf("ListUpdate failure: %v", err)
 		db.setError(err)
-		// backoff strategy: MIN((2**N-1 * 15 minutes) * (RAND + 1), 24 hours)
-		n := 1 << db.updateAPIErrors
-		delay := time.Duration(float64(n) * (rand.Float64() + 1) * float64(baseRetryDelay))
-		if delay > maxRetryDelay {
-			delay = maxRetryDelay
-		}
-		db.updateAPIErrors++
-		return delay, false
+		// Retry updating every minute in case of error. Error cases are handled
+		// by the API itself.
+		return 1 * time.Minute, false
 	}
-	db.updateAPIErrors = 0
 
 	// add jitter to wait time to avoid all servers lining up
 	nextUpdateWait := db.config.UpdatePeriod + time.Duration(rand.Int31n(60)-30)*time.Second

database_test.go

@@ -552,38 +552,15 @@ func TestDatabaseUpdate(t *testing.T) {
 		t.Fatalf("update 5, database state mismatch:\ngot  %+v\nwant %+v", gotDB, wantDB)
 	}
 
-	// Update 6: api is broken for some unknown reason. Checks the backoff
+	// Update 6: api is broken for some unknown reason. Checks retrial delay.
 	errResponse = errors.New("Something broke")
 	delay, updated = db.Update(context.Background(), mockAPI)
 	if db.err == nil || updated {
 		t.Fatalf("update 6, unexpected update success")
 	}
-	minDelay := baseRetryDelay.Seconds() * float64(1) * float64(1)
-	maxDelay := baseRetryDelay.Seconds() * float64(2) * float64(1)
-	if delay.Seconds() < minDelay || delay.Seconds() > maxDelay {
-		t.Fatalf("update 6, Expected delay %v to be between %v and %v", delay.Seconds(), minDelay, maxDelay)
-	}
-
-	// Update 7: api is still broken, check backoff is larger
-	delay, updated = db.Update(context.Background(), mockAPI)
-	if db.err == nil || updated {
-		t.Fatalf("update 7, unexpected update success")
-	}
-	minDelay = baseRetryDelay.Seconds() * float64(1) * float64(2)
-	maxDelay = baseRetryDelay.Seconds() * float64(2) * float64(2)
-	if delay.Seconds() < minDelay || delay.Seconds() > maxDelay {
-		t.Fatalf("update 7, Expected delay %v to be between %v and %v", delay.Seconds(), minDelay, maxDelay)
-	}
-
-	// Update 8: api is still broken, check that backoff is larger than before
-	delay, updated = db.Update(context.Background(), mockAPI)
-	if db.err == nil || updated {
-		t.Fatalf("update 8, unexpected update success")
-	}
-	minDelay = baseRetryDelay.Seconds() * float64(1) * float64(4)
-	maxDelay = baseRetryDelay.Seconds() * float64(2) * float64(4)
-	if delay.Seconds() < minDelay || delay.Seconds() > maxDelay {
-		t.Fatalf("update 8, Expected delay %v to be between %v and %v", delay.Seconds(), minDelay, maxDelay)
+	expectedDelay = 1 * time.Minute
+	if delay != expectedDelay {
+		t.Fatalf("update 6, Expected delay %v to be %v", delay.Seconds(), expectedDelay.Seconds)
 	}
 }