Skip to content

Commit

Permalink
Merge pull request #2933 from cloudflare/kenton/sqlite-foreign-constr…
Browse files Browse the repository at this point in the history
…aint

SQLite: Make foreign key errors on txn commit visible to app.
  • Loading branch information
kentonv authored Oct 17, 2024
2 parents 507e965 + bcda208 commit 19e1596
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/workerd/api/sql-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ export default {
// Test defer_foreign_keys (explodes the DO)
await assert.rejects(async () => {
await doReq('sql-test-foreign-keys');
}, /Error: internal error/);
}, /constraints were violated: FOREIGN KEY constraint failed: SQLITE_CONSTRAINT/);

// Some increments.
assert.equal(await doReq('increment'), 1);
Expand Down
3 changes: 3 additions & 0 deletions src/workerd/io/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ wd_cc_library(
"actor-sqlite.h",
"actor-storage.h",
],
implementation_deps = [
"@sqlite3",
],
visibility = ["//visibility:public"],
deps = [
":actor-storage_capnp",
Expand Down
17 changes: 17 additions & 0 deletions src/workerd/io/actor-sqlite.c++
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <workerd/jsg/exception.h>
#include <workerd/util/sentry.h>

#include <sqlite3.h>

#include <algorithm>

namespace workerd {
Expand Down Expand Up @@ -648,6 +650,21 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::onNoPendingFlush() {
return outputGate.wait();
}

void ActorSqlite::TxnCommitRegulator::onError(
kj::Maybe<int> sqliteErrorCode, kj::StringPtr message) const {
KJ_IF_SOME(c, sqliteErrorCode) {
if (c == SQLITE_CONSTRAINT) {
JSG_ASSERT(false, Error,
"Durable Object was reset and rolled back to its last known good state because the "
"application left the database in a state where constraints were violated: ",
message);
}
}

// For any other type of error, fall back to the default behavior (throwing a non-JSG exception)
// as we don't know for sure that the problem is the application's fault.
}

const ActorSqlite::Hooks ActorSqlite::Hooks::DEFAULT = ActorSqlite::Hooks{};

kj::Promise<void> ActorSqlite::Hooks::scheduleRun(kj::Maybe<kj::Date> newAlarmTime) {
Expand Down
10 changes: 9 additions & 1 deletion src/workerd/io/actor-sqlite.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,16 @@ class ActorSqlite final: public ActorCacheInterface, private kj::TaskSet::ErrorH
SqliteKv kv;
SqliteMetadata metadata;

// Define a SqliteDatabase::Regulator that is similar to TRUSTED but turns certain SQLite errors
// into application errors as appropriate when committing an implicit transaction.
class TxnCommitRegulator: public SqliteDatabase::Regulator {
public:
void onError(kj::Maybe<int> sqliteErrorCode, kj::StringPtr message) const;
};
static constexpr TxnCommitRegulator TRUSTED_TXN_COMMIT;

SqliteDatabase::Statement beginTxn = db->prepare("BEGIN TRANSACTION");
SqliteDatabase::Statement commitTxn = db->prepare("COMMIT TRANSACTION");
SqliteDatabase::Statement commitTxn = db->prepare(TRUSTED_TXN_COMMIT, "COMMIT TRANSACTION");

kj::Maybe<kj::Exception> broken;

Expand Down

0 comments on commit 19e1596

Please sign in to comment.