Move Existing SQLite Sentry routing to exception details

src/workerd/io/BUILD.bazel

@@ -502,6 +502,7 @@ kj_test(
     deps = [
         ":actor",
         ":io-gate",
+        "//src/workerd/util:sentry",
         "//src/workerd/util:test",
         "//src/workerd/util:test-util",
         "@sqlite3",

src/workerd/io/actor-sqlite-test.c++

@@ -6,6 +6,7 @@
 #include "io-gate.h"
 
 #include <workerd/util/capnp-mock.h>
+#include <workerd/util/sentry.h>
 #include <workerd/util/test.h>
 
 #include <sqlite3.h>
@@ -2602,7 +2603,9 @@ KJ_TEST("sync() throws after critical error in explicit transaction") {
     KJ_FAIL_ASSERT("Query should have failed with SQLITE_NOMEM");
   } catch (kj::Exception& e) {
     // Expected: out of memory error. We catch and ignore this to continue the test.
-    KJ_ASSERT(e.getDescription().contains("SENTRY_DO"));
+    KJ_ASSERT(!e.getDescription().contains("SENTRY_DO"));
+    auto sentryTag = KJ_ASSERT_NONNULL(e.getDetail(SENTRY_TAG_DETAIL_ID));
+    KJ_ASSERT(sentryTag.asChars() == "SENTRY_DO"_kj);
     KJ_ASSERT(e.getDescription().contains("out of memory"));
   }
 

src/workerd/util/BUILD.bazel

@@ -431,6 +431,7 @@ kj_test(
     size = "large",
     src = "sqlite-test.c++",
     deps = [
+        ":sentry",
         ":sqlite",
         "//src/workerd/io:io-gate",
         "@sqlite3",

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

@@ -337,6 +337,13 @@ void doLockTest(bool walMode) {
   auto dir = kj::newInMemoryDirectory(kj::nullClock());
   SqliteDatabase::Vfs vfs(*dir);
 
+  auto expectBusy = [](const kj::Exception& e) {
+    KJ_EXPECT(e.getDescription().contains("database is locked"), e);
+    KJ_EXPECT(!e.getDescription().contains("NOSENTRY"), e);
+    auto sentryTag = KJ_ASSERT_NONNULL(e.getDetail(SENTRY_TAG_DETAIL_ID));
+    KJ_EXPECT(sentryTag.asChars() == "NOSENTRY"_kj, e);
+  };
+
   SqliteDatabase db(vfs, kj::Path({"foo"}), kj::WriteMode::CREATE | kj::WriteMode::MODIFY);
 
   if (walMode) {
@@ -376,15 +383,15 @@ void doLockTest(bool walMode) {
   {
     // Arrange for two threads to increment in a loop simultaneously. Eventually one will fail with
     // a conflict.
-    kj::Thread thread([&vfs = vfs, &stop, &counter]() noexcept {
+    kj::Thread thread([&vfs = vfs, &stop, &counter, &expectBusy]() noexcept {
       KJ_DEFER(stop.store(true, std::memory_order_relaxed););
       SqliteDatabase db2(vfs, kj::Path({"foo"}), kj::WriteMode::MODIFY);
       while (!stop.load(std::memory_order_relaxed)) {
         KJ_IF_SOME(e, kj::runCatchingExceptions([&]() {
           db2.run(INCREMENT);
           counter.fetch_add(1, std::memory_order_relaxed);
         })) {
-          KJ_EXPECT(e.getDescription().contains("database is locked"), e);
+          expectBusy(e);
           break;
         }
       }
@@ -398,7 +405,7 @@ void doLockTest(bool walMode) {
           db.run(INCREMENT);
           counter.fetch_add(1, std::memory_order_relaxed);
         })) {
-          KJ_EXPECT(e.getDescription().contains("database is locked"), e);
+          expectBusy(e);
           break;
         }
       }

src/workerd/util/sqlite.c++

@@ -190,14 +190,14 @@ kj::String dbErrorMessage(int errorCode, sqlite3* db) {
 // exceptions through SQLite.
 static thread_local kj::Maybe<kj::Exception>* vfsErrorListener = nullptr;
 
-void tagDoSentry(kj::Exception& e) {
+void tagSentry(kj::Exception& e, kj::StringPtr tag) {
   if (e.getDetail(SENTRY_TAG_DETAIL_ID) == kj::none) {
-    e.setDetail(SENTRY_TAG_DETAIL_ID, kj::heapArray("SENTRY_DO"_kj.asBytes()));
+    e.setDetail(SENTRY_TAG_DETAIL_ID, kj::heapArray(tag.asBytes()));
   }
 }
 
-[[noreturn]] void throwDoSentryException(kj::Exception&& e) {
-  tagDoSentry(e);
+[[noreturn]] void throwSentryException(kj::Exception&& e, kj::StringPtr tag) {
+  tagSentry(e, tag);
   kj::throwFatalException(kj::mv(e));
 }
 
@@ -209,7 +209,7 @@ void tagDoSentry(kj::Exception& e) {
 // only the frames between the throw and the catch. We actually want to retain the full trace
 // through SQLite.
 void reportVfsErrorCaught(kj::Exception&& e) {
-  tagDoSentry(e);
+  tagSentry(e, "SENTRY_DO"_kj);
   if (vfsErrorListener != nullptr) {
     // Only capture the first error; assume subsequent errors are side effects.
     if (*vfsErrorListener == kj::none) {
@@ -259,20 +259,26 @@ class SqliteCallScope {
 // the return value of sqlite3_errmsg() or a string literal containing a similarly
 // application-approriate error message. A reference called `regulator` must be in-scope.
 // sqliteErrorCode is a kj::Maybe<int> and represents the error code from sqlite.
-#define SQLITE_REQUIRE(condition, sqliteErrorCode, errorMessage, ...)                              \
+#define SQLITE_REQUIRE_WITH_TAG(condition, sqliteErrorCode, sentryTag, errorMessage, ...)          \
   if (!(condition)) {                                                                              \
     regulator->onError(sqliteErrorCode, errorMessage);                                             \
-    KJ_FAIL_REQUIRE("SENTRY_DO SQLite failed", errorMessage, ##__VA_ARGS__);                       \
+    throwSentryException(                                                                          \
+        KJ_EXCEPTION(FAILED, "SQLite failed", errorMessage, ##__VA_ARGS__), sentryTag);            \
   }
 
+#define SQLITE_REQUIRE(condition, sqliteErrorCode, errorMessage, ...)                              \
+  SQLITE_REQUIRE_WITH_TAG(condition, sqliteErrorCode, "SENTRY_DO"_kj, errorMessage, ##__VA_ARGS__)
+
 // Make a SQLite call and check the returned error code. Use this version when the call is not
 // associated with an open DB connection.
 #define SQLITE_CALL_NODB(code, ...)                                                                \
   do {                                                                                             \
     int _ec = code;                                                                                \
     if (_ec != SQLITE_OK) {                                                                        \
-      throwDoSentryException(KJ_EXCEPTION(                                                         \
-          FAILED, kj::str(sqlite3_errstr(_ec), ": ", namedErrorCode(_ec)), ##__VA_ARGS__));        \
+      throwSentryException(                                                                        \
+          KJ_EXCEPTION(                                                                            \
+              FAILED, kj::str(sqlite3_errstr(_ec), ": ", namedErrorCode(_ec)), ##__VA_ARGS__),     \
+          "SENTRY_DO"_kj);                                                                         \
     }                                                                                              \
   } while (false)
 
@@ -299,8 +305,8 @@ class SqliteCallScope {
     KJ_ASSERT(error != SQLITE_MISUSE, "SQLite misused: " code, ##__VA_ARGS__);                     \
     handleCriticalError(error, dbErrorMessage(error, db), sqliteCallScope.getException());         \
     if (error == SQLITE_IOERR) sqliteCallScope.rethrowVfsError();                                  \
-    SQLITE_REQUIRE(error != SQLITE_BUSY, error, kj::str("NOSENTRY ", dbErrorMessage(error, db)),   \
-        ##__VA_ARGS__);                                                                            \
+    SQLITE_REQUIRE_WITH_TAG(                                                                       \
+        error != SQLITE_BUSY, error, "NOSENTRY"_kj, dbErrorMessage(error, db), ##__VA_ARGS__);     \
     SQLITE_REQUIRE(error == SQLITE_OK, error, dbErrorMessage(error, db), ##__VA_ARGS__);           \
   } while (false);