Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

Commit 53e5257

Browse files
authored
feat: do not rethrow RuntimeStatusError from a Commit() mutator (#1320)
Treat the exception thrown by the mutator as if the enclosed `Status` had been returned instead (OK is converted to an "unknown" status). This allows the transaction to be rerun (when the status is transient).
1 parent 42a0779 commit 53e5257

File tree

3 files changed

+34
-1
lines changed

3 files changed

+34
-1
lines changed

google/cloud/spanner/client.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ StatusOr<CommitResult> Client::Commit(
178178
#endif
179179
mutations = mutator(txn);
180180
#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
181+
} catch (RuntimeStatusError const& error) {
182+
// Treat this like mutator() returned a bad Status.
183+
Status status = error.status();
184+
if (status.ok()) {
185+
status = Status(StatusCode::kUnknown, "OK Status thrown from mutator");
186+
}
187+
mutations = status;
181188
} catch (...) {
182189
auto rb_status = Rollback(txn);
183190
if (!RerunnablePolicy::IsOk(rb_status)) {

google/cloud/spanner/client.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,9 @@ class Client {
461461
* @param backoff_policy controls how long `Commit` waits between reruns.
462462
*
463463
* @throw Rethrows any exception thrown by @p `mutator` (after rolling back
464-
* the transaction).
464+
* the transaction). However, a `RuntimeStatusError` exception is
465+
* instead consumed and converted into a `mutator` return value of the
466+
* enclosed `Status`.
465467
*/
466468
StatusOr<CommitResult> Commit(
467469
std::function<StatusOr<Mutations>(Transaction)> const& mutator,

google/cloud/spanner/client_test.cc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,30 @@ TEST(ClientTest, CommitMutatorException) {
584584
FAIL();
585585
}
586586
}
587+
588+
TEST(ClientTest, CommitMutatorRuntimeStatusException) {
589+
auto conn = std::make_shared<MockConnection>();
590+
EXPECT_CALL(*conn, Rollback(_)).WillRepeatedly(Return(Status()));
591+
Client client(conn);
592+
try {
593+
auto result = client.Commit([](Transaction const&) -> StatusOr<Mutations> {
594+
throw RuntimeStatusError(Status()); // OK
595+
});
596+
EXPECT_EQ(StatusCode::kUnknown, result.status().code());
597+
EXPECT_THAT(result.status().message(), HasSubstr("OK Status thrown"));
598+
} catch (...) {
599+
FAIL(); // exception consumed
600+
}
601+
try {
602+
auto result = client.Commit([](Transaction const&) -> StatusOr<Mutations> {
603+
throw RuntimeStatusError(Status(StatusCode::kCancelled, "uh oh"));
604+
});
605+
EXPECT_EQ(StatusCode::kCancelled, result.status().code());
606+
EXPECT_EQ(result.status().message(), "uh oh");
607+
} catch (...) {
608+
FAIL(); // exception consumed
609+
}
610+
}
587611
#endif
588612

589613
TEST(ClientTest, CommitMutatorRerunTransientFailures) {

0 commit comments

Comments
 (0)