storage: add retry for transient network issues

Adds retry for connection reset and connection
refused errors for most operations (except for writes).

Splits invoke test into pre/post go1.10 versions in
order to not touch old code that is no longer covered
by CI.

Fixes #1638

Change-Id: Ie8f9857a49e1b4399b125dd9567db6d18b1d1dd8
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/55970
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tyler Bui-Palsulich <tbp@google.com>
Reviewed-by: Frank Natividad <franknatividad@google.com>

storage/go110.go

@@ -16,14 +16,30 @@
 
 package storage
 
-import "google.golang.org/api/googleapi"
+import (
+	"net/url"
+	"strings"
+
+	"google.golang.org/api/googleapi"
+)
 
 func shouldRetry(err error) bool {
 	switch e := err.(type) {
 	case *googleapi.Error:
 		// Retry on 429 and 5xx, according to
 		// https://cloud.google.com/storage/docs/exponential-backoff.
 		return e.Code == 429 || (e.Code >= 500 && e.Code < 600)
+	case *url.Error:
+		// Retry socket-level errors ECONNREFUSED and ENETUNREACH (from syscall).
+		// Unfortunately the error type is unexported, so we resort to string
+		// matching.
+		retriable := []string{"connection refused", "connection reset"}
+		for _, s := range retriable {
+			if strings.Contains(e.Error(), s) {
+				return true
+			}
+		}
+		return false
 	case interface{ Temporary() bool }:
 		return e.Temporary()
 	default:

storage/go110_test.go

@@ -0,0 +1,60 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build go1.10
+
+package storage
+
+import (
+	"context"
+	"errors"
+	"net/url"
+	"testing"
+
+	"google.golang.org/api/googleapi"
+)
+
+func TestInvoke(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	// Time-based tests are flaky. We just make sure that invoke eventually
+	// returns with the right error.
+
+	for _, test := range []struct {
+		count      int   // Number of times to return retryable error.
+		initialErr error // Error to return initially.
+		finalErr   error // Error to return after count returns of retryCode.
+	}{
+		{0, &googleapi.Error{Code: 0}, nil},
+		{0, &googleapi.Error{Code: 0}, errors.New("foo")},
+		{1, &googleapi.Error{Code: 429}, nil},
+		{1, &googleapi.Error{Code: 429}, errors.New("bar")},
+		{2, &googleapi.Error{Code: 518}, nil},
+		{2, &googleapi.Error{Code: 599}, &googleapi.Error{Code: 428}},
+		{1, &url.Error{Op: "blah", URL: "blah", Err: errors.New("connection refused")}, nil},
+	} {
+		counter := 0
+		call := func() error {
+			counter++
+			if counter <= test.count {
+				return test.initialErr
+			}
+			return test.finalErr
+		}
+		got := runWithRetry(ctx, call)
+		if got != test.finalErr {
+			t.Errorf("%+v: got %v, want %v", test, got, test.finalErr)
+		}
+	}
+}

storage/invoke_test.go → storage/not_go110_test.go

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// +build !go1.10
+
 package storage
 
 import (