Do not throw exceptions in WriteObject().

google/cloud/storage/client.cc

@@ -59,6 +59,20 @@ ObjectReadStream Client::ReadObjectImpl(
   return ObjectReadStream(*std::move(streambuf));
 }
 
+ObjectWriteStream Client::WriteObjectImpl(
+    internal::InsertObjectStreamingRequest const& request) {
+  auto streambuf = raw_client_->WriteObject(request);
+  if (!streambuf) {
+    ObjectWriteStream error_stream(google::cloud::internal::make_unique<
+                                   internal::ObjectWriteErrorStreambuf>(
+        std::move(streambuf).status()));
+    error_stream.setstate(std::ios::badbit | std::ios::eofbit);
+    error_stream.Close();
+    return error_stream;
+  }
+  return ObjectWriteStream(*std::move(streambuf));
+}
+
 bool Client::UseSimpleUpload(std::string const& file_name) const {
   auto status = google::cloud::internal::status(file_name);
   if (!is_regular(status)) {

google/cloud/storage/client.h

@@ -892,7 +892,7 @@ class Client {
                               Options&&... options) {
     internal::ReadObjectRangeRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    return ReadObjectImpl(std::move(request));
+    return ReadObjectImpl(request);
   }
 
   /**
@@ -958,7 +958,7 @@ class Client {
                                 Options&&... options) {
     internal::InsertObjectStreamingRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    return ObjectWriteStream(raw_client_->WriteObject(request).value());
+    return WriteObjectImpl(request);
   }
 
   /**
@@ -2786,6 +2786,9 @@ class Client {
   ObjectReadStream ReadObjectImpl(
       internal::ReadObjectRangeRequest const& request);
 
+  ObjectWriteStream WriteObjectImpl(
+      internal::InsertObjectStreamingRequest const& request);
+
   // The version of UploadFile() where UseResumableUploadSession is one of the
   // options. Note how this does not use InsertObjectMedia at all.
   template <typename... Options>

google/cloud/storage/client_write_object_test.cc

@@ -83,6 +83,8 @@ TEST_F(WriteObjectTest, WriteObject) {
             EXPECT_CALL(*mock_result, DoClose())
                 .WillRepeatedly(Return(internal::HttpResponse{200, text, {}}));
             EXPECT_CALL(*mock_result, IsOpen()).WillRepeatedly(Return(true));
+            EXPECT_CALL(*mock_result, ValidateHash(_))
+                .WillRepeatedly(Return(true));
             std::unique_ptr<internal::ObjectWriteStreambuf> result(mock_result);
             return make_status_or(std::move(result));
           }));
@@ -102,54 +104,29 @@ TEST_F(WriteObjectTest, WriteObjectTooManyFailures) {
     return StatusOr<std::unique_ptr<internal::ObjectWriteStreambuf>>(
         TransientError());
   };
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   EXPECT_CALL(*mock, WriteObject(_))
       .WillOnce(Invoke(returner))
       .WillOnce(Invoke(returner))
       .WillOnce(Invoke(returner));
-  EXPECT_THROW(
-      try {
-        client.WriteObject("test-bucket-name", "test-object-name").Close();
-      } catch (std::runtime_error const& ex) {
-        EXPECT_THAT(ex.what(), HasSubstr("Retry policy exhausted"));
-        EXPECT_THAT(ex.what(), HasSubstr("WriteObject"));
-        throw;
-      },
-      std::runtime_error);
-#else
-  // With EXPECT_DEATH*() the mocking framework cannot detect how many times the
-  // operation is called.
-  EXPECT_CALL(*mock, WriteObject(_)).WillRepeatedly(Invoke(returner));
-  EXPECT_DEATH_IF_SUPPORTED(
-      client.WriteObject("test-bucket-name", "test-object-name").Close(),
-      "exceptions are disabled");
-#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+
+  auto stream = client.WriteObject("test-bucket-name", "test-object-name");
+  EXPECT_TRUE(stream.bad());
+  EXPECT_FALSE(stream.metadata().status().ok());
+  EXPECT_EQ(TransientError().code(), stream.metadata().status().code())
+      << ", status=" << stream.metadata().status();
 }
 
 TEST_F(WriteObjectTest, WriteObjectPermanentFailure) {
   auto returner = [](internal::InsertObjectStreamingRequest const&) {
     return StatusOr<std::unique_ptr<internal::ObjectWriteStreambuf>>(
         PermanentError());
   };
-#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   EXPECT_CALL(*mock, WriteObject(_)).WillOnce(Invoke(returner));
-  EXPECT_THROW(
-      try {
-        client->WriteObject("test-bucket-name", "test-object-name").Close();
-      } catch (std::runtime_error const& ex) {
-        EXPECT_THAT(ex.what(), HasSubstr("Permanent error"));
-        EXPECT_THAT(ex.what(), HasSubstr("WriteObject"));
-        throw;
-      },
-      std::runtime_error);
-#else
-  // With EXPECT_DEATH*() the mocking framework cannot detect how many times the
-  // operation is called.
-  EXPECT_CALL(*mock, WriteObject(_)).WillRepeatedly(Invoke(returner));
-  EXPECT_DEATH_IF_SUPPORTED(
-      client->WriteObject("test-bucket-name", "test-object-name").Close(),
-      "exceptions are disabled");
-#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+  auto stream = client->WriteObject("test-bucket-name", "test-object-name");
+  EXPECT_TRUE(stream.bad());
+  EXPECT_FALSE(stream.metadata().status().ok());
+  EXPECT_EQ(PermanentError().code(), stream.metadata().status().code())
+      << ", status=" << stream.metadata().status();
 }
 
 }  // namespace

google/cloud/storage/internal/curl_client_test.cc

@@ -195,11 +195,6 @@ TEST_P(CurlClientTest, InsertObjectMediaSimple) {
 }
 
 TEST_P(CurlClientTest, InsertObjectMediaMultipart) {
-  std::string const error_type = GetParam();
-  if (error_type != "credentials-failure") {
-    // TODO(#1735) - enable this test when ObjectWriteStream uses StatusOr.
-    return;
-  }
   auto actual = client_
                     ->InsertObjectMedia(
                         InsertObjectMediaRequest("bkt", "obj", "contents"))

google/cloud/storage/tests/object_media_integration_test.cc

@@ -692,7 +692,7 @@ TEST_F(ObjectMediaIntegrationTest, ReadRangeXml) {
   EXPECT_STATUS_OK(status);
 }
 
-TEST_F(ObjectMediaIntegrationTest, ConnectionFailureJSON) {
+TEST_F(ObjectMediaIntegrationTest, ConnectionFailureReadJSON) {
   Client client{ClientOptions(oauth2::CreateAnonymousCredentials())
                     .set_endpoint("http://localhost:0"),
                 LimitedErrorCountRetryPolicy(2)};
@@ -713,7 +713,7 @@ TEST_F(ObjectMediaIntegrationTest, ConnectionFailureJSON) {
       << ", status=" << stream.status();
 }
 
-TEST_F(ObjectMediaIntegrationTest, ConnectionFailureXML) {
+TEST_F(ObjectMediaIntegrationTest, ConnectionFailureReadXML) {
   google::cloud::internal::SetEnv("CLOUD_STORAGE_TESTBENCH_ENDPOINT",
                                   "http://localhost:0");
   Client client{ClientOptions(oauth2::CreateAnonymousCredentials())
@@ -732,6 +732,81 @@ TEST_F(ObjectMediaIntegrationTest, ConnectionFailureXML) {
       << ", status=" << stream.status();
 }
 
+TEST_F(ObjectMediaIntegrationTest, ConnectionFailureWriteJSON) {
+  Client client{ClientOptions(oauth2::CreateAnonymousCredentials())
+                    .set_endpoint("http://localhost:0"),
+                LimitedErrorCountRetryPolicy(2)};
+
+  std::string bucket_name = flag_bucket_name;
+  auto object_name = MakeRandomObjectName();
+
+  // We force the library to use the JSON API by adding the
+  // `IfGenerationNotMatch()` parameter, both JSON and XML use the same code to
+  // download, but controlling the endpoint for JSON is easier.
+  auto stream = client.WriteObject(
+      bucket_name, object_name, IfGenerationMatch(0), IfGenerationNotMatch(7));
+  EXPECT_TRUE(stream.bad());
+  EXPECT_FALSE(stream.metadata().status().ok());
+  EXPECT_EQ(StatusCode::kUnavailable, stream.metadata().status().code())
+      << ", status=" << stream.metadata().status();
+}
+
+TEST_F(ObjectMediaIntegrationTest, ConnectionFailureWriteXML) {
+  google::cloud::internal::SetEnv("CLOUD_STORAGE_TESTBENCH_ENDPOINT",
+                                  "http://localhost:0");
+  Client client{ClientOptions(oauth2::CreateAnonymousCredentials())
+                    .set_endpoint("http://localhost:0"),
+                LimitedErrorCountRetryPolicy(2)};
+
+  std::string bucket_name = flag_bucket_name;
+  auto object_name = MakeRandomObjectName();
+
+  auto stream = client.WriteObject(
+      bucket_name, object_name, IfGenerationMatch(0), IfGenerationNotMatch(7));
+  EXPECT_TRUE(stream.bad());
+  EXPECT_FALSE(stream.metadata().status().ok());
+  EXPECT_EQ(StatusCode::kUnavailable, stream.metadata().status().code())
+      << ", status=" << stream.metadata().status();
+}
+
+TEST_F(ObjectMediaIntegrationTest, ConnectionFailureDownloadFile) {
+  google::cloud::internal::SetEnv("CLOUD_STORAGE_TESTBENCH_ENDPOINT",
+                                  "http://localhost:0");
+  Client client{ClientOptions(oauth2::CreateAnonymousCredentials())
+                    .set_endpoint("http://localhost:0"),
+                LimitedErrorCountRetryPolicy(2)};
+
+  std::string bucket_name = flag_bucket_name;
+  auto object_name = MakeRandomObjectName();
+  auto file_name = MakeRandomObjectName();
+
+  Status status = client.DownloadToFile(bucket_name, object_name, file_name);
+  EXPECT_FALSE(status.ok());
+  EXPECT_EQ(StatusCode::kUnavailable, status.code()) << ", status=" << status;
+}
+
+TEST_F(ObjectMediaIntegrationTest, ConnectionFailureUploadFile) {
+  google::cloud::internal::SetEnv("CLOUD_STORAGE_TESTBENCH_ENDPOINT",
+                                  "http://localhost:0");
+  Client client{ClientOptions(oauth2::CreateAnonymousCredentials())
+                    .set_endpoint("http://localhost:0"),
+                LimitedErrorCountRetryPolicy(2)};
+
+  std::string bucket_name = flag_bucket_name;
+  auto object_name = MakeRandomObjectName();
+  auto file_name = MakeRandomObjectName();
+
+  std::ofstream(file_name) << LoremIpsum();
+
+  StatusOr<ObjectMetadata> meta =
+      client.UploadFile(file_name, bucket_name, object_name);
+  EXPECT_FALSE(meta.ok()) << "value=" << meta.value();
+  EXPECT_EQ(StatusCode::kUnavailable, meta.status().code())
+      << ", status=" << meta.status();
+
+  EXPECT_EQ(0, std::remove(file_name.c_str()));
+}
+
 }  // anonymous namespace
 }  // namespace STORAGE_CLIENT_NS
 }  // namespace storage

google/cloud/storage/tests/object_resumable_write_integration_test.cc

@@ -81,15 +81,12 @@ TEST_F(ObjectResumableWriteIntegrationTest, WriteWithContentTypeFailure) {
   std::ostringstream expected;
 
   // Create the object, but only if it does not exist already.
-  TestPermanentFailure([&] {
-    auto os = client->WriteObject(
-        bucket_name, object_name, IfGenerationMatch(0),
-        WithObjectMetadata(ObjectMetadata().set_content_type("text/plain")));
-    os.exceptions(std::ios_base::failbit);
-    os << LoremIpsum();
-    os.Close();
-    ObjectMetadata meta = os.metadata().value();
-  });
+  auto os = client->WriteObject(
+      bucket_name, object_name, IfGenerationMatch(0),
+      WithObjectMetadata(ObjectMetadata().set_content_type("text/plain")));
+  EXPECT_TRUE(os.bad());
+  EXPECT_FALSE(os.metadata().status().ok())
+      << ", status=" << os.metadata().status();
 }
 
 TEST_F(ObjectResumableWriteIntegrationTest, WriteWithUseResumable) {