diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 957dff9ae70fee..972b08546e36ae 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1763,21 +1763,27 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { Local filterFunc = filterValue.As(); - context.filterCallback = [env, - filterFunc](std::string_view item) -> bool { - // TODO(@jasnell): The use of ToLocalChecked here means that if - // the filter function throws an error the process will crash. - // The filterCallback should be updated to avoid the check and - // propagate the error correctly. - Local argv[] = { - String::NewFromUtf8(env->isolate(), - item.data(), - NewStringType::kNormal, - static_cast(item.size())) - .ToLocalChecked()}; - Local result = - filterFunc->Call(env->context(), Null(env->isolate()), 1, argv) - .ToLocalChecked(); + context.filterCallback = [&](std::string_view item) -> bool { + // If there was an error in the previous call to the filter's + // callback, we skip calling it again. + if (db->ignore_next_sqlite_error_) { + return false; + } + + Local argv[1]; + if (!ToV8Value(env->context(), item, env->isolate()) + .ToLocal(&argv[0])) { + db->SetIgnoreNextSQLiteError(true); + return false; + } + + Local result; + if (!filterFunc->Call(env->context(), Null(env->isolate()), 1, argv) + .ToLocal(&result)) { + db->SetIgnoreNextSQLiteError(true); + return false; + } + return result->BooleanValue(env->isolate()); }; } @@ -2239,9 +2245,11 @@ Local StatementExecutionHelper::Get(Environment* env, LocalVector keys(isolate); keys.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { - MaybeLocal key = ColumnNameToName(env, stmt, i); - if (key.IsEmpty()) return Undefined(isolate); - keys.emplace_back(key.ToLocalChecked()); + Local key; + if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) { + return Undefined(isolate); + } + keys.emplace_back(key); } DCHECK_EQ(keys.size(), row_values.size()); @@ -2755,12 +2763,8 @@ BaseObjectPtr SQLTagStore::PrepareStatement( if (stmt == nullptr) { sqlite3_stmt* s = nullptr; - Local sql_str = - String::NewFromUtf8(isolate, sql.c_str()).ToLocalChecked(); - Utf8Value sql_utf8(isolate, sql_str); - int r = sqlite3_prepare_v2( - session->database_->connection_, *sql_utf8, -1, &s, 0); + session->database_->connection_, sql.c_str(), -1, &s, 0); if (r != SQLITE_OK) { THROW_ERR_SQLITE_ERROR(isolate, "Failed to prepare statement"); diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js index 21271cec05996e..934ef576bc93fa 100644 --- a/test/parallel/test-sqlite-session.js +++ b/test/parallel/test-sqlite-session.js @@ -366,6 +366,30 @@ suite('conflict resolution', () => { }); }); +test('filter handler throws', (t) => { + const database1 = new DatabaseSync(':memory:'); + const database2 = new DatabaseSync(':memory:'); + const createTableSql = 'CREATE TABLE data1(key INTEGER PRIMARY KEY); CREATE TABLE data2(key INTEGER PRIMARY KEY);'; + database1.exec(createTableSql); + database2.exec(createTableSql); + + const session = database1.createSession(); + + database1.exec('INSERT INTO data1 (key) VALUES (1), (2), (3)'); + database1.exec('INSERT INTO data2 (key) VALUES (1), (2), (3), (4), (5)'); + + t.assert.throws(() => { + database2.applyChangeset(session.changeset(), { + filter: (tableName) => { + throw new Error(`Error filtering table ${tableName}`); + } + }); + }, { + name: 'Error', + message: 'Error filtering table data1' + }); +}); + test('database.createSession() - filter changes', (t) => { const database1 = new DatabaseSync(':memory:'); const database2 = new DatabaseSync(':memory:');