fix(storage): retry net.OpError on connection reset (#10154) · googleapis/google-cloud-go@54fab10 · GitHub

googleapis / google-cloud-go Public

• Notifications You must be signed in to change notification settings
• Fork 1.6k
• Star 4.5k

• Code
• Issues 278
• Pull requests 151
• Actions
• Wiki
• Security and quality
• Insights

Additional navigation options

Commit 54fab10

Browse filesBrowse the repository at this point in the historyBrowse files

tritone

authored

fix(storage): retry net.OpError on connection reset (#10154)

We are seeing these errors surfaced via net.OpError as well as url.Error. Update the ShouldRetry function accordingly. Also, use net.ErrClosed sentinel over string matching. Fixes #9478

1 parent 7c52978 commit 54fab10
Copy full SHA for 54fab10

2 files changed

+11-9Lines changed: 11 additions & 9 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• storage
  • invoke.go
  • invoke_test.go

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎storage/invoke.go‎

Copy file name to clipboardExpand all lines: storage/invoke.go

+4-6Lines changed: 4 additions & 6 deletions

Original file line number	Diff line number	Diff line change
@@ -105,18 +105,16 @@ func ShouldRetry(err error) bool {
105	105	if errors.Is(err, io.ErrUnexpectedEOF) {
106	106	return true
107	107	}
	108	+ if errors.Is(err, net.ErrClosed) {
	109	+ return true
	110	+ }
108	111
109	112	switch e := err.(type) {
110		- case *net.OpError:
111		- if strings.Contains(e.Error(), "use of closed network connection") {
112		- // TODO: check against net.ErrClosed (go 1.16+) instead of string
113		- return true
114		- }
115	113	case *googleapi.Error:
116	114	// Retry on 408, 429, and 5xx, according to
117	115	// https://cloud.google.com/storage/docs/exponential-backoff.
118	116	return e.Code == 408 || e.Code == 429 || (e.Code >= 500 && e.Code < 600)
119		- case *url.Error:
	117	+ case *net.OpError, *url.Error:
120	118	// Retry socket-level errors ECONNREFUSED and ECONNRESET (from syscall).
121	119	// Unfortunately the error type is unexported, so we resort to string
122	120	// matching.

Collapse file

‎storage/invoke_test.go‎

Copy file name to clipboardExpand all lines: storage/invoke_test.go

+7-3Lines changed: 7 additions & 3 deletions

Original file line number	Diff line number	Diff line change
@@ -346,6 +346,11 @@ func TestShouldRetry(t *testing.T) {
346	346	inputErr: &url.Error{Op: "blah", URL: "blah", Err: errors.New("connection refused")},
347	347	shouldRetry: true,
348	348	},
	349	+ {
	350	+ desc: "net.OpError{Err: errors.New(\"connection reset by peer\")}",
	351	+ inputErr: &net.OpError{Op: "blah", Net: "tcp", Err: errors.New("connection reset by peer")},
	352	+ shouldRetry: true,
	353	+ },
349	354	{
350	355	desc: "io.ErrUnexpectedEOF",
351	356	inputErr: io.ErrUnexpectedEOF,
@@ -382,9 +387,8 @@ func TestShouldRetry(t *testing.T) {
382	387	shouldRetry: false,
383	388	},
384	389	{
385		- desc: "wrapped ErrClosed text",
386		- // TODO: check directly against wrapped net.ErrClosed (go 1.16+)
387		- inputErr: &net.OpError{Op: "write", Err: errors.New("use of closed network connection")},
	390	+ desc: "wrapped net.ErrClosed",
	391	+ inputErr: &net.OpError{Err: net.ErrClosed},
388	392	shouldRetry: true,
389	393	},
390	394	} {

0 commit comments

Comments

0 (0)