Don't check the response body close error in the client

If we successfully read the response all the way to EOF and decode it, that constitutes a successful RPC. If the body close happens to return a non-nil error, the client shouldn't care, since it was able to fully read the response.
twitchtv · Nov 15, 2021 · e0e9a4d · e0e9a4d
1 parent c4c8032
commit e0e9a4d
Show file tree

Hide file tree

Showing 17 changed files with 16 additions and 165 deletions.
diff --git a/clientcompat/internal/clientcompat/clientcompat.twirp.go b/clientcompat/internal/clientcompat/clientcompat.twirp.go
diff --git a/example/service.twirp.go b/example/service.twirp.go
diff --git a/internal/twirptest/client_test.go b/internal/twirptest/client_test.go
@@ -16,7 +16,6 @@ package twirptest
 import (
 	"context"
 	"fmt"
-	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -681,55 +680,3 @@ func (c *wrappedHTTPClient) Do(req *http.Request) (resp *http.Response, err erro
 	c.wasCalled = true
 	return c.client.Do(req)
 }
-
-func TestClientReturnsCloseErrors(t *testing.T) {
-	h := PickyHatmaker(1)
-	s := httptest.NewServer(NewHaberdasherServer(h, nil))
-	defer s.Close()
-
-	httpClient := &bodyCloseErrClient{base: http.DefaultClient}
-
-	testcase := func(client Haberdasher) func(*testing.T) {
-		return func(t *testing.T) {
-			_, err := client.MakeHat(context.Background(), &Size{Inches: 1})
-			if err == nil {
-				t.Error("expected an error when body fails to close, have nil")
-			} else {
-				if errors.Cause(err) != bodyCloseErr {
-					t.Errorf("got wrong root cause for error, have=%v, want=%v", err, bodyCloseErr)
-				}
-			}
-		}
-	}
-	t.Run("json client", testcase(NewHaberdasherJSONClient(s.URL, httpClient)))
-	t.Run("protobuf client", testcase(NewHaberdasherProtobufClient(s.URL, httpClient)))
-}
-
-// bodyCloseErrClient implements HTTPClient, but the response bodies it returns
-// give an error when they are closed.
-type bodyCloseErrClient struct {
-	base HTTPClient
-}
-
-func (c *bodyCloseErrClient) Do(req *http.Request) (*http.Response, error) {
-	resp, err := c.base.Do(req)
-	if resp == nil {
-		return resp, err
-	}
-	resp.Body = &errBodyCloser{resp.Body}
-	return resp, nil
-}
-
-var bodyCloseErr = errors.New("failed closing")
-
-type errBodyCloser struct {
-	base io.ReadCloser
-}
-
-func (ec *errBodyCloser) Read(p []byte) (int, error) {
-	return ec.base.Read(p)
-}
-
-func (ec *errBodyCloser) Close() error {
-	return bodyCloseErr
-}
diff --git a/internal/twirptest/empty_service/empty_service.twirp.go b/internal/twirptest/empty_service/empty_service.twirp.go
diff --git a/internal/twirptest/google_protobuf_imports/service.twirp.go b/internal/twirptest/google_protobuf_imports/service.twirp.go
diff --git a/internal/twirptest/importable/importable.twirp.go b/internal/twirptest/importable/importable.twirp.go
diff --git a/internal/twirptest/importer/importer.twirp.go b/internal/twirptest/importer/importer.twirp.go
diff --git a/internal/twirptest/importer_local/importer_local.twirp.go b/internal/twirptest/importer_local/importer_local.twirp.go
diff --git a/internal/twirptest/importmapping/x/x.twirp.go b/internal/twirptest/importmapping/x/x.twirp.go
diff --git a/internal/twirptest/json_serialization/json_serialization.twirp.go b/internal/twirptest/json_serialization/json_serialization.twirp.go
diff --git a/internal/twirptest/multiple/multiple1.twirp.go b/internal/twirptest/multiple/multiple1.twirp.go
diff --git a/internal/twirptest/no_package_name/no_package_name.twirp.go b/internal/twirptest/no_package_name/no_package_name.twirp.go
diff --git a/internal/twirptest/no_package_name_importer/no_package_name_importer.twirp.go b/internal/twirptest/no_package_name_importer/no_package_name_importer.twirp.go
diff --git a/internal/twirptest/service.twirp.go b/internal/twirptest/service.twirp.go
diff --git a/internal/twirptest/service_method_same_name/service_method_same_name.twirp.go b/internal/twirptest/service_method_same_name/service_method_same_name.twirp.go
diff --git a/internal/twirptest/snake_case_names/snake_case_names.twirp.go b/internal/twirptest/snake_case_names/snake_case_names.twirp.go
diff --git a/protoc-gen-twirp/generator.go b/protoc-gen-twirp/generator.go
@@ -794,13 +794,7 @@ func (t *twirp) generateUtils() {
 	t.P(`  if err != nil {`)
 	t.P(`    return ctx, wrapInternal(err, "failed to do request")`)
 	t.P(`  }`)
-	t.P()
-	t.P(`  defer func() {`)
-	t.P(`    cerr := resp.Body.Close()`)
-	t.P(`    if err == nil && cerr != nil {`)
-	t.P(`      err = wrapInternal(cerr, "failed to close response body")`)
-	t.P(`    }`)
-	t.P(`  }()`)
+	t.P(`  defer func() { _ = resp.Body.Close() }()`)
 	t.P()
 	t.P(`  if err = ctx.Err(); err != nil {`)
 	t.P(`    return ctx, wrapInternal(err, "aborted because context was done")`)