Skip to content

Commit d017d77

Browse files
committed
sqlite: error handling in DatabaseSync.prototype.applyChangeset()
1 parent 25842c5 commit d017d77

File tree

1 file changed

+80
-29
lines changed

1 file changed

+80
-29
lines changed

src/node_sqlite.cc

Lines changed: 80 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,30 @@ void JSValueToSQLiteResult(Isolate* isolate,
201201
}
202202
}
203203

204+
inline int CREATE_INTERNAL_SAVEPOINT(sqlite3* db, const char* name) {
205+
const std::string command =
206+
std::string("SAVEPOINT __nodesqlite_") + name;
207+
return sqlite3_exec(db, command.c_str(), nullptr, nullptr, nullptr);
208+
}
209+
210+
inline int RELEASE_INTERNAL_SAVEPOINT_ON_SUCCESS(sqlite3* db,
211+
const char* name) {
212+
const std::string command =
213+
std::string("RELEASE SAVEPOINT __nodesqlite_") + name;
214+
return sqlite3_exec(db, command.c_str(), nullptr, nullptr, nullptr);
215+
}
216+
217+
inline int RELEASE_INTERNAL_SAVEPOINT_ON_FAILURE(sqlite3* db,
218+
const char* name) {
219+
const std::string command =
220+
std::string("ROLLBACK TRANSACTION TO SAVEPOINT __nodesqlite_") + name;
221+
int r = sqlite3_exec(db, command.c_str(), nullptr, nullptr, nullptr);
222+
if (r != SQLITE_OK) {
223+
return r;
224+
}
225+
return RELEASE_INTERNAL_SAVEPOINT_ON_SUCCESS(db, name);
226+
}
227+
204228
class DatabaseSync;
205229

206230
inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, DatabaseSync* db) {
@@ -1541,6 +1565,8 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
15411565
conflictCallback = nullptr;
15421566
filterCallback = nullptr;
15431567

1568+
bool filterCallbackThrew = false;
1569+
15441570
DatabaseSync* db;
15451571
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
15461572
Environment* env = Environment::GetCurrent(args);
@@ -1577,12 +1603,10 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
15771603
Local<Function> conflictFunc = conflictValue.As<Function>();
15781604
conflictCallback = [env, conflictFunc](int conflictType) -> int {
15791605
Local<Value> argv[] = {Integer::New(env->isolate(), conflictType)};
1580-
TryCatch try_catch(env->isolate());
1581-
Local<Value> result =
1582-
conflictFunc->Call(env->context(), Null(env->isolate()), 1, argv)
1583-
.FromMaybe(Local<Value>());
1584-
if (try_catch.HasCaught()) {
1585-
try_catch.ReThrow();
1606+
Local<Value> result;
1607+
if (!conflictFunc->Call(env->context(), Null(env->isolate()), 1, argv)
1608+
.ToLocal(&result)) {
1609+
// An error will have been scheduled.
15861610
return SQLITE_CHANGESET_ABORT;
15871611
}
15881612
constexpr auto invalid_value = -1;
@@ -1591,19 +1615,13 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
15911615
};
15921616
}
15931617

1594-
bool hasIt;
1595-
if (!options->HasOwnProperty(env->context(), env->filter_string())
1596-
.To(&hasIt)) {
1618+
Local<Value> filterValue;
1619+
if (!options->Get(env->context(), env->filter_string())
1620+
.ToLocal(&filterValue)) {
1621+
// An error will have been scheduled.
15971622
return;
15981623
}
1599-
if (hasIt) {
1600-
Local<Value> filterValue;
1601-
if (!options->Get(env->context(), env->filter_string())
1602-
.ToLocal(&filterValue)) {
1603-
// An error will have been scheduled.
1604-
return;
1605-
}
1606-
1624+
if (!filterValue->IsUndefined()) {
16071625
if (!filterValue->IsFunction()) {
16081626
THROW_ERR_INVALID_ARG_TYPE(
16091627
env->isolate(),
@@ -1613,23 +1631,47 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
16131631

16141632
Local<Function> filterFunc = filterValue.As<Function>();
16151633

1616-
filterCallback = [env, filterFunc](std::string item) -> bool {
1617-
// TODO(@jasnell): The use of ToLocalChecked here means that if
1618-
// the filter function throws an error the process will crash.
1619-
// The filterCallback should be updated to avoid the check and
1620-
// propagate the error correctly.
1621-
Local<Value> argv[] = {String::NewFromUtf8(env->isolate(),
1622-
item.c_str(),
1623-
NewStringType::kNormal)
1624-
.ToLocalChecked()};
1625-
Local<Value> result =
1626-
filterFunc->Call(env->context(), Null(env->isolate()), 1, argv)
1627-
.ToLocalChecked();
1634+
filterCallback =
1635+
[env, &filterCallbackThrew, filterFunc](std::string item) -> bool {
1636+
if (filterCallbackThrew) {
1637+
// Do not call the user function again if a previous call already
1638+
// threw an exception.
1639+
return false;
1640+
}
1641+
1642+
Local<Value> tableName;
1643+
if (!String::NewFromUtf8(env->isolate(),
1644+
item.c_str(),
1645+
NewStringType::kNormal)
1646+
.ToLocal(&tableName)) {
1647+
filterCallbackThrew = true;
1648+
return false;
1649+
}
1650+
1651+
Local<Value> result;
1652+
if (!filterFunc->Call(env->context(),
1653+
Null(env->isolate()),
1654+
1,
1655+
&tableName)
1656+
.ToLocal(&result)) {
1657+
filterCallbackThrew = true;
1658+
return false;
1659+
}
1660+
16281661
return result->BooleanValue(env->isolate());
16291662
};
16301663
}
16311664
}
16321665

1666+
constexpr const char* savepoint_name = "applychangeset";
1667+
int create_savepoint_ret =
1668+
CREATE_INTERNAL_SAVEPOINT(db->connection_, savepoint_name);
1669+
CHECK_ERROR_OR_THROW(env->isolate(),
1670+
db,
1671+
create_savepoint_ret,
1672+
SQLITE_OK,
1673+
void());
1674+
16331675
ArrayBufferViewContents<uint8_t> buf(args[0]);
16341676
int r = sqlite3changeset_apply(
16351677
db->connection_,
@@ -1638,15 +1680,24 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
16381680
xFilter,
16391681
xConflict,
16401682
nullptr);
1683+
1684+
if (filterCallbackThrew) {
1685+
RELEASE_INTERNAL_SAVEPOINT_ON_FAILURE(db->connection_, savepoint_name);
1686+
return;
1687+
}
16411688
if (r == SQLITE_OK) {
1689+
RELEASE_INTERNAL_SAVEPOINT_ON_SUCCESS(db->connection_, savepoint_name);
16421690
args.GetReturnValue().Set(true);
16431691
return;
16441692
}
16451693
if (r == SQLITE_ABORT) {
16461694
// this is not an error, return false
1695+
RELEASE_INTERNAL_SAVEPOINT_ON_SUCCESS(db->connection_, savepoint_name);
16471696
args.GetReturnValue().Set(false);
16481697
return;
16491698
}
1699+
1700+
RELEASE_INTERNAL_SAVEPOINT_ON_FAILURE(db->connection_, savepoint_name);
16501701
THROW_ERR_SQLITE_ERROR(env->isolate(), r);
16511702
}
16521703

0 commit comments

Comments
 (0)