Replace defer w.Close() with explicit error checking

cloudtest/gcsfake/gcsfake.go

@@ -57,6 +57,7 @@ type BucketHandle struct {
 	ObjAttrs       []*storage.ObjectAttrs // Objects that will be returned by iterator
 	Objs           map[string]*ObjectHandle
 	WritesMustFail bool
+	ClosesMustFail bool
 }
 
 // NewBucketHandle creates a new empty BucketHandle.
@@ -83,6 +84,7 @@ func (bh *BucketHandle) Object(name string) stiface.ObjectHandle {
 		Bucket:         bh,
 		Data:           new(bytes.Buffer),
 		WritesMustFail: bh.WritesMustFail,
+		ClosesMustFail: bh.ClosesMustFail,
 	}
 }
 
@@ -138,6 +140,7 @@ type ObjectHandle struct {
 	Bucket         *BucketHandle
 	Data           *bytes.Buffer
 	WritesMustFail bool
+	ClosesMustFail bool
 }
 
 // NewReader returns a fakeReader for this ObjectHandle.
@@ -150,17 +153,19 @@ func (o *ObjectHandle) NewReader(context.Context) (stiface.Reader, error) {
 // NewWriter returns a fakeWrite for this ObjectHandle.
 func (o *ObjectHandle) NewWriter(context.Context) stiface.Writer {
 	return &fakeWriter{
-		object:   o,
-		buf:      o.Data,
-		mustFail: o.WritesMustFail,
+		object:        o,
+		buf:           o.Data,
+		mustFail:      o.WritesMustFail,
+		closeMustFail: o.ClosesMustFail,
 	}
 }
 
 type fakeWriter struct {
 	stiface.Writer
-	object   *ObjectHandle
-	buf      *bytes.Buffer
-	mustFail bool
+	object        *ObjectHandle
+	buf           *bytes.Buffer
+	mustFail      bool
+	closeMustFail bool
 }
 
 // Write writes data to the fake bucket. The object is created if it does not
@@ -173,6 +178,9 @@ func (w *fakeWriter) Write(p []byte) (int, error) {
 	return w.buf.Write(p)
 }
 func (w *fakeWriter) Close() error {
+	if w.closeMustFail {
+		return errors.New("close failed")
+	}
 	return nil
 }
 

cloudtest/gcsfake/gcsfake_test.go

@@ -196,6 +196,11 @@ func Test_fakeWriter_Close(t *testing.T) {
 		t.Errorf("Close() returned error: %v", err)
 	}
 
+	w.closeMustFail = true
+	if err := w.Close(); err == nil {
+		t.Errorf("Close() did not return an error")
+	}
+
 }
 
 func Test_fakeReader_Read(t *testing.T) {

uploader/uploader.go

@@ -26,11 +26,17 @@ func New(client stiface.Client, bucket string) *Uploader {
 func (u *Uploader) Upload(ctx context.Context, path string, content []byte) (stiface.ObjectHandle, error) {
 	obj := u.bucket.Object(path)
 	w := obj.NewWriter(ctx)
-	defer w.Close()
 
 	_, err := io.Copy(w, bytes.NewBuffer(content))
 	if err != nil {
 		return nil, err
 	}
+
+	// Avoid using defer w.Close() here as it would hide errors occurring
+	// while closing the writer, such as permission errors.
+	err = w.Close()
+	if err != nil {
+		return nil, err
+	}
 	return obj, nil
 }

uploader/uploader_test.go

@@ -24,8 +24,11 @@ func TestUploader_Upload(t *testing.T) {
 	client := &gcsfake.GCSClient{}
 	failingBucket := gcsfake.NewBucketHandle()
 	failingBucket.WritesMustFail = true
+	failingCloseBucket := gcsfake.NewBucketHandle()
+	failingCloseBucket.ClosesMustFail = true
 	client.AddTestBucket("test_bucket", gcsfake.NewBucketHandle())
 	client.AddTestBucket("failing_bucket", failingBucket)
+	client.AddTestBucket("failing_close_bucket", failingCloseBucket)
 	type args struct {
 		ctx     context.Context
 		path    string
@@ -59,6 +62,17 @@ func TestUploader_Upload(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name:   "close-fails",
+			client: client,
+			bucket: "failing_close_bucket",
+			args: args{
+				ctx:     context.Background(),
+				content: []byte("failing close"),
+				path:    "this/is/a/test",
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {