FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

sqlite: replace `ToLocalChecked` and improve filter error handling · nodejs/node@d949222 · GitHub

/ node Public

Commit d949222

Browse files
geeksilva97
authored and
targos
committed
sqlite: replace ToLocalChecked and improve filter error handling
PR-URL: #60028 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent c412b18 commit d949222

2 files changed

Lines changed: 51 additions & 23 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,21 +1763,27 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
17631763

17641764
Local<Function> filterFunc = filterValue.As<Function>();
17651765

1766-
context.filterCallback = [env,
1767-
filterFunc](std::string_view item) -> bool {
1768-
// TODO(@jasnell): The use of ToLocalChecked here means that if
1769-
// the filter function throws an error the process will crash.
1770-
// The filterCallback should be updated to avoid the check and
1771-
// propagate the error correctly.
1772-
Local<Value> argv[] = {
1773-
String::NewFromUtf8(env->isolate(),
1774-
item.data(),
1775-
NewStringType::kNormal,
1776-
static_cast<int>(item.size()))
1777-
.ToLocalChecked()};
1778-
Local<Value> result =
1779-
filterFunc->Call(env->context(), Null(env->isolate()), 1, argv)
1780-
.ToLocalChecked();
1766+
context.filterCallback = [&](std::string_view item) -> bool {
1767+
// If there was an error in the previous call to the filter's
1768+
// callback, we skip calling it again.
1769+
if (db->ignore_next_sqlite_error_) {
1770+
return false;
1771+
}
1772+
1773+
Local<Value> argv[1];
1774+
if (!ToV8Value(env->context(), item, env->isolate())
1775+
.ToLocal(&argv[0])) {
1776+
db->SetIgnoreNextSQLiteError(true);
1777+
return false;
1778+
}
1779+
1780+
Local<Value> result;
1781+
if (!filterFunc->Call(env->context(), Null(env->isolate()), 1, argv)
1782+
.ToLocal(&result)) {
1783+
db->SetIgnoreNextSQLiteError(true);
1784+
return false;
1785+
}
1786+
17811787
return result->BooleanValue(env->isolate());
17821788
};
17831789
}
@@ -2239,9 +2245,11 @@ Local<Value> StatementExecutionHelper::Get(Environment* env,
22392245
LocalVector<Name> keys(isolate);
22402246
keys.reserve(num_cols);
22412247
for (int i = 0; i < num_cols; ++i) {
2242-
MaybeLocal<Name> key = ColumnNameToName(env, stmt, i);
2243-
if (key.IsEmpty()) return Undefined(isolate);
2244-
keys.emplace_back(key.ToLocalChecked());
2248+
Local<Name> key;
2249+
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
2250+
return Undefined(isolate);
2251+
}
2252+
keys.emplace_back(key);
22452253
}
22462254

22472255
DCHECK_EQ(keys.size(), row_values.size());
@@ -2755,12 +2763,8 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
27552763

27562764
if (stmt == nullptr) {
27572765
sqlite3_stmt* s = nullptr;
2758-
Local<String> sql_str =
2759-
String::NewFromUtf8(isolate, sql.c_str()).ToLocalChecked();
2760-
Utf8Value sql_utf8(isolate, sql_str);
2761-
27622766
int r = sqlite3_prepare_v2(
2763-
session->database_->connection_, *sql_utf8, -1, &s, 0);
2767+
session->database_->connection_, sql.c_str(), -1, &s, 0);
27642768

27652769
if (r != SQLITE_OK) {
27662770
THROW_ERR_SQLITE_ERROR(isolate, "Failed to prepare statement");

‎test/parallel/test-sqlite-session.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,30 @@ suite('conflict resolution', () => {
366366
});
367367
});
368368

369+
test('filter handler throws', (t) => {
370+
const database1 = new DatabaseSync(':memory:');
371+
const database2 = new DatabaseSync(':memory:');
372+
const createTableSql = 'CREATE TABLE data1(key INTEGER PRIMARY KEY); CREATE TABLE data2(key INTEGER PRIMARY KEY);';
373+
database1.exec(createTableSql);
374+
database2.exec(createTableSql);
375+
376+
const session = database1.createSession();
377+
378+
database1.exec('INSERT INTO data1 (key) VALUES (1), (2), (3)');
379+
database1.exec('INSERT INTO data2 (key) VALUES (1), (2), (3), (4), (5)');
380+
381+
t.assert.throws(() => {
382+
database2.applyChangeset(session.changeset(), {
383+
filter: (tableName) => {
384+
throw new Error(`Error filtering table ${tableName}`);
385+
}
386+
});
387+
}, {
388+
name: 'Error',
389+
message: 'Error filtering table data1'
390+
});
391+
});
392+
369393
test('database.createSession() - filter changes', (t) => {
370394
const database1 = new DatabaseSync(':memory:');
371395
const database2 = new DatabaseSync(':memory:');

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL