Merge pull request #5363 from cloudflare/jmp/fix-clang-tidy-miserror

Refactor SqliteKv’s multi-put to silence an errant clang-tidy lint

Author: justin-mp

src/workerd/util/sqlite-kv.c++

@@ -171,4 +171,15 @@ void SqliteKv::beforeSqliteReset() {
   tableCreated = false;
 }
 
+void SqliteKv::rollbackMultiPut(Initialized& stmts, WriteOptions options) {
+  KJ_IF_SOME(e, kj::runCatchingExceptions([&]() {
+    // This should be rare, so we don't prepare a statement for it.
+    stmts.db.run({.regulator = stmts.regulator, .allowUnconfirmed = options.allowUnconfirmed},
+        kj::str("ROLLBACK TO _cf_put_multiple_savepoint"));
+    stmts.stmtMultiPutRelease.run({.allowUnconfirmed = options.allowUnconfirmed});
+  })) {
+    KJ_LOG(WARNING, "silencing exception encountered while rolling back multi-put", e);
+  }
+}
+
 }  // namespace workerd

src/workerd/util/sqlite-kv.h

@@ -177,6 +177,10 @@ class SqliteKv: private SqliteDatabase::ResetListener {
   // first write.
 
   void beforeSqliteReset() override;
+
+  // Helper function that rolls back a multi-put statement and swallows any exceptions that may
+  // occur during the rollback.
+  void rollbackMultiPut(Initialized& stmts, WriteOptions options);
 };
 
 // Iterator over list results.
@@ -272,20 +276,13 @@ void SqliteKv::put(ArrayOfKeyValuePair& pairs, WriteOptions options) {
   // general structure can be shared somehow?
   auto& stmts = ensureInitialized(options.allowUnconfirmed);
   stmts.stmtMultiPutSavepoint.run({.allowUnconfirmed = options.allowUnconfirmed});
-  for (const auto& pair: pairs) {
-    try {
+
+  {
+    // If any of the puts throw an exception, rollback the transaction and re-throw the exception
+    // from the put that failed.
+    KJ_ON_SCOPE_FAILURE(rollbackMultiPut(stmts, options));
+    for (const auto& pair: pairs) {
       put(pair.key, pair.value, {.allowUnconfirmed = options.allowUnconfirmed});
-    } catch (...) {
-      try {
-        // This should be rare, so we don't prepare a statement for it.
-        stmts.db.run({.regulator = stmts.regulator, .allowUnconfirmed = options.allowUnconfirmed},
-            kj::str("ROLLBACK TO _cf_put_multiple_savepoint"));
-        stmts.stmtMultiPutRelease.run({.allowUnconfirmed = options.allowUnconfirmed});
-      } catch (...) {
-        auto e = kj::getCaughtExceptionAsKj();
-        KJ_LOG(WARNING, "silencing exception encountered while rolling back multi-put", e);
-      }
-      throw;
     }
   }
   stmts.stmtMultiPutRelease.run({.allowUnconfirmed = options.allowUnconfirmed});