fix(nodejs): swallow errors thrown from add progress callback

This commit is contained in:
Brendan Clement
2026-05-18 09:14:25 -07:00
parent aed71c3bc0
commit cb811428f9
2 changed files with 34 additions and 1 deletions

View File

@@ -140,6 +140,23 @@ describe.each([arrow15, arrow16, arrow17, arrow18])(
}
});
it("should swallow errors thrown from the progress callback", async () => {
const warn = jest
.spyOn(console, "warn")
.mockImplementation(() => undefined);
try {
const res = await table.add([{ id: 1 }, { id: 2 }], {
progress: () => {
throw new Error("callback bomb");
},
});
expect(res.version).toBeGreaterThan(0);
expect(warn).toHaveBeenCalled();
} finally {
warn.mockRestore();
}
});
it("should let me close the table", async () => {
expect(table.isOpen()).toBe(true);
table.close();

View File

@@ -92,6 +92,9 @@ export interface AddDataOptions {
* callback is slow, intermediate updates may be dropped to avoid stalling
* the write.
*
* Errors thrown from the callback are logged with `console.warn` and
* swallowed — they do not abort the write.
*
* @example
* ```ts
* await table.add(data, {
@@ -751,7 +754,20 @@ export class LocalTable extends Table {
const schema = await this.schema();
const buffer = await fromDataToBuffer(data, undefined, schema);
return await this.inner.add(buffer, mode, options?.progress);
// Wrap the user callback so a thrown error doesn't surface as an
// unhandled exception (the callback fires from a napi threadsafe
// function — exceptions there crash the process).
const userProgress = options?.progress;
const progress = userProgress
? (p: WriteProgress) => {
try {
userProgress(p);
} catch (e) {
console.warn("Table.add progress callback threw:", e);
}
}
: undefined;
return await this.inner.add(buffer, mode, progress);
}
async update(