diff --git a/src/workerd/io/BUILD.bazel b/src/workerd/io/BUILD.bazel index cc41ab6ed49..6d3a3cabe64 100644 --- a/src/workerd/io/BUILD.bazel +++ b/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", diff --git a/src/workerd/io/actor-sqlite-test.c++ b/src/workerd/io/actor-sqlite-test.c++ index 502e5df03c0..e099b96e7e4 100644 --- a/src/workerd/io/actor-sqlite-test.c++ +++ b/src/workerd/io/actor-sqlite-test.c++ @@ -6,6 +6,7 @@ #include "io-gate.h" #include +#include #include #include @@ -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")); } diff --git a/src/workerd/util/BUILD.bazel b/src/workerd/util/BUILD.bazel index 8d8589ce5ea..c837b270dfc 100644 --- a/src/workerd/util/BUILD.bazel +++ b/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", diff --git a/src/workerd/util/sqlite-test.c++ b/src/workerd/util/sqlite-test.c++ index 928051a31a7..0f304a8755e 100644 --- a/src/workerd/util/sqlite-test.c++ +++ b/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,7 +383,7 @@ 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)) { @@ -384,7 +391,7 @@ void doLockTest(bool walMode) { 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; } } diff --git a/src/workerd/util/sqlite.c++ b/src/workerd/util/sqlite.c++ index 9a05922dc03..dd50d10d006 100644 --- a/src/workerd/util/sqlite.c++ +++ b/src/workerd/util/sqlite.c++ @@ -190,14 +190,14 @@ kj::String dbErrorMessage(int errorCode, sqlite3* db) { // exceptions through SQLite. static thread_local kj::Maybe* 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 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);