feat: MinSpeed -> MinBytesPerSecond, check every 3 seconds

Changelog.md

@@ -32,6 +32,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 ### Fixed
 * Use correct binary name when finding location of addcert.scpt
 * GODT-267 Correctly detect if a message is a draft even if does not have DraftLabel
+* GODT-308 reduce minimum read speed threshold to avoid issues with flaky internet
 
 ## [v1.2.6] Donghai - beta (2020-03-31)
 

pkg/config/pmapi_prod.go

@@ -31,11 +31,11 @@ import (
 
 func (c *Config) GetAPIConfig() *pmapi.ClientConfig {
 	return &pmapi.ClientConfig{
-		AppVersion:       strings.Title(c.appName) + "_" + c.version,
-		ClientID:         c.appName,
-		Timeout:          10 * time.Minute, // Overall request timeout (~25MB / 10 mins => ~40kB/s, should be reasonable).
-		FirstReadTimeout: 30 * time.Second, // 30s to match 30s response header timeout.
-		MinSpeed:         1 << 10,          // Enforce minimum download speed of 1kB/s.
+		AppVersion:        strings.Title(c.appName) + "_" + c.version,
+		ClientID:          c.appName,
+		Timeout:           10 * time.Minute, // Overall request timeout (~25MB / 10 mins => ~40kB/s, should be reasonable).
+		FirstReadTimeout:  30 * time.Second, // 30s to match 30s response header timeout.
+		MinBytesPerSecond: 1 << 10,          // Enforce minimum download speed of 1kB/s.
 	}
 }
 

pkg/pmapi/client.go

@@ -87,13 +87,13 @@ type ClientConfig struct {
 	Timeout time.Duration
 
 	// FirstReadTimeout specifies the timeout from getting response to the first read of body response.
-	// This timeout is applied only when MinSpeed is used.
+	// This timeout is applied only when MinBytesPerSecond is used.
 	// Default is 5 minutes.
 	FirstReadTimeout time.Duration
 
-	// MinSpeed specifies minimum Bytes per second or the request will be canceled.
+	// MinBytesPerSecond specifies minimum Bytes per second or the request will be canceled.
 	// Zero means no limitation.
-	MinSpeed int64
+	MinBytesPerSecond int64
 }
 
 // Client defines the interface of a PMAPI client.
@@ -316,7 +316,7 @@ func (c *client) doJSONBuffered(req *http.Request, reqBodyBuffer []byte, data in
 	req.Header.Set("Accept", "application/vnd.protonmail.v1+json")
 
 	var cancelRequest context.CancelFunc
-	if c.cm.config.MinSpeed > 0 {
+	if c.cm.config.MinBytesPerSecond > 0 {
 		var ctx context.Context
 		ctx, cancelRequest = context.WithCancel(req.Context())
 		defer func() {
@@ -332,7 +332,7 @@ func (c *client) doJSONBuffered(req *http.Request, reqBodyBuffer []byte, data in
 	defer res.Body.Close() //nolint[errcheck]
 
 	var resBody []byte
-	if c.cm.config.MinSpeed == 0 {
+	if c.cm.config.MinBytesPerSecond == 0 {
 		resBody, err = ioutil.ReadAll(res.Body)
 	} else {
 		resBody, err = c.readAllMinSpeed(res.Body, cancelRequest)
@@ -417,17 +417,22 @@ func (c *client) readAllMinSpeed(data io.Reader, cancelRequest context.CancelFun
 	timer := time.AfterFunc(firstReadTimeout, func() {
 		cancelRequest()
 	})
+
+	// speedCheckSeconds controls how often we check the transfer speed.
+	const speedCheckSeconds = 3
+
 	var buffer bytes.Buffer
 	for {
-		_, err := io.CopyN(&buffer, data, c.cm.config.MinSpeed)
+		_, err := io.CopyN(&buffer, data, c.cm.config.MinBytesPerSecond*speedCheckSeconds)
 		timer.Stop()
-		timer.Reset(1 * time.Second)
+		timer.Reset(speedCheckSeconds * time.Second)
 		if err == io.EOF {
 			break
 		} else if err != nil {
 			return nil, err
 		}
 	}
+
 	return ioutil.ReadAll(&buffer)
 }
 

pkg/pmapi/client_test.go

@@ -30,10 +30,10 @@ import (
 )
 
 var testClientConfig = &ClientConfig{
-	AppVersion:       "GoPMAPI_1.0.14",
-	ClientID:         "demoapp",
-	FirstReadTimeout: 500 * time.Millisecond,
-	MinSpeed:         256,
+	AppVersion:        "GoPMAPI_1.0.14",
+	ClientID:          "demoapp",
+	FirstReadTimeout:  500 * time.Millisecond,
+	MinBytesPerSecond: 256,
 }
 
 func newTestClient(cm *ClientManager) *client {
@@ -175,7 +175,7 @@ func TestClient_FirstReadTimeout(t *testing.T) {
 
 func TestClient_MinSpeedTimeout(t *testing.T) {
 	finish, c := newTestServerCallbacks(t,
-		routeSlow(2*time.Second),
+		routeSlow(4*time.Second), // 1 second longer than the minimum transfer speed poll time.
 	)
 	defer finish()