feat(napi): add async variants that keep the Node event loop free (#337)
* feat(napi): add processPdfAsync, classifyPdfAsync, extractPagesMarkdownAsync The Node bindings are synchronous, so every call parses on the event loop thread — up to hundreds of milliseconds of dead loop per document in a server. Add additive AsyncTask-based variants that run the same shared implementations on the libuv thread pool and return promises. The existing synchronous exports keep their names, signatures, and behaviour; each sync/async pair shares one implementation. Panics in compute() are caught and surfaced as rejections, matching the sync error contract. Closes #336 Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com> * fix(napi): read async task buffers in place instead of copying Review feedback on #337: buffer.to_vec() copied the whole PDF on the event loop before the task was queued, so large inputs still stalled the loop and doubled peak memory. The tasks now hold the napi Buffer itself — its ref pins the JS allocation for the task's lifetime and the backing store is stable, so compute() reads it directly from the worker thread. Callers must not mutate the buffer until the promise settles (same contract as Node's async fs APIs); documented on each export and in the README. The suggested removal of ts_return_type was checked and rejected: without it napi-rs generates Promise<unknown> for AsyncTask returns. A comment now records that finding. Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com> * fix(napi): copy async task input on the JS thread for soundness Review feedback on #337: holding the napi Buffer and reading it from the libuv worker was unsound. Buffer derefs straight to the JS-side allocation, so a caller mutating it before the promise settled would race the worker's reads — undefined behavior, not a recoverable error, and the documented don't-mutate contract was unenforceable. Deferring the copy to compute() would not help: any off-thread read races the same way. The JS thread is the only race-free place to take the copy, because JS is single-threaded and nothing can mutate the buffer during the synchronous part of the call. Revert to an owned Vec<u8> copied at call time. The cost is one memcpy, negligible next to the parse the async variants exist to unblock. Docs now state the buffer may be reused or mutated immediately, and a test locks in the copy semantics by mutating the input while a parse is in flight. Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
This commit is contained in:
co-authored by
Abimael Martell
Cursor Agent
parent
1f28c00a13
commit
fabec0aec3
@@ -2,13 +2,16 @@ import { readFileSync } from 'fs';
|
||||
import { strict as assert } from 'assert';
|
||||
import {
|
||||
processPdf,
|
||||
processPdfAsync,
|
||||
detectPdf,
|
||||
classifyPdf,
|
||||
classifyPdfAsync,
|
||||
extractText,
|
||||
extractTextWithPositions,
|
||||
extractTextInRegions,
|
||||
detectVectorGridInRegion,
|
||||
extractPagesMarkdown,
|
||||
extractPagesMarkdownAsync,
|
||||
} from './index.js';
|
||||
|
||||
const fixture = readFileSync('../tests/fixtures/thermo-freon12.pdf');
|
||||
@@ -124,10 +127,75 @@ assert.equal(picked.pages[0].page, 2);
|
||||
assert.equal(picked.pages[1].page, 0);
|
||||
console.log(' extractPagesMarkdown with pages: OK');
|
||||
|
||||
// --- Async variants ---
|
||||
console.log('Testing async variants...');
|
||||
|
||||
// processPdfAsync returns a promise and matches the sync result
|
||||
const asyncResultPromise = processPdfAsync(fixture);
|
||||
assert.ok(asyncResultPromise instanceof Promise);
|
||||
const asyncResult = await asyncResultPromise;
|
||||
assert.equal(asyncResult.pdfType, result.pdfType);
|
||||
assert.equal(asyncResult.pageCount, result.pageCount);
|
||||
assert.equal(asyncResult.markdown, result.markdown);
|
||||
console.log(' processPdfAsync: OK');
|
||||
|
||||
// processPdfAsync with pages
|
||||
const asyncResult2 = await processPdfAsync(fixture, [1]);
|
||||
assert.equal(asyncResult2.markdown, result2.markdown);
|
||||
console.log(' processPdfAsync with pages: OK');
|
||||
|
||||
// classifyPdfAsync matches the sync result
|
||||
const asyncClassified = await classifyPdfAsync(fixture);
|
||||
assert.equal(asyncClassified.pdfType, classified.pdfType);
|
||||
assert.equal(asyncClassified.pageCount, classified.pageCount);
|
||||
assert.equal(asyncClassified.confidence, classified.confidence);
|
||||
assert.deepEqual(asyncClassified.pagesNeedingOcr, classified.pagesNeedingOcr);
|
||||
console.log(' classifyPdfAsync: OK');
|
||||
|
||||
// extractPagesMarkdownAsync matches the sync result
|
||||
const asyncAllPages = await extractPagesMarkdownAsync(fixture);
|
||||
assert.equal(asyncAllPages.pages.length, allPages.pages.length);
|
||||
assert.deepEqual(
|
||||
asyncAllPages.pages.map(p => p.markdown),
|
||||
allPages.pages.map(p => p.markdown),
|
||||
);
|
||||
assert.equal(asyncAllPages.isComplex, allPages.isComplex);
|
||||
console.log(' extractPagesMarkdownAsync: OK');
|
||||
|
||||
// selected pages preserve caller order
|
||||
const asyncPicked = await extractPagesMarkdownAsync(fixture, [2, 0]);
|
||||
assert.equal(asyncPicked.pages.length, 2);
|
||||
assert.equal(asyncPicked.pages[0].page, 2);
|
||||
assert.equal(asyncPicked.pages[1].page, 0);
|
||||
console.log(' extractPagesMarkdownAsync with pages: OK');
|
||||
|
||||
// input buffer is copied at call time: mutating it immediately after the
|
||||
// call must not affect the in-flight parse
|
||||
const scratch = Buffer.from(fixture);
|
||||
const inFlight = processPdfAsync(scratch);
|
||||
scratch.fill(0);
|
||||
const fromMutated = await inFlight;
|
||||
assert.equal(fromMutated.markdown, result.markdown);
|
||||
console.log(' processPdfAsync input copied at call time: OK');
|
||||
|
||||
// concurrent async calls all settle
|
||||
const [c1, c2, c3] = await Promise.all([
|
||||
processPdfAsync(fixture),
|
||||
classifyPdfAsync(fixture),
|
||||
extractPagesMarkdownAsync(fixture),
|
||||
]);
|
||||
assert.equal(c1.pdfType, 'TextBased');
|
||||
assert.equal(c2.pdfType, 'TextBased');
|
||||
assert.equal(c3.pages.length, 3);
|
||||
console.log(' concurrent async calls: OK');
|
||||
|
||||
// --- Error handling ---
|
||||
console.log('Testing error handling...');
|
||||
assert.throws(() => processPdf(Buffer.from('not a pdf')), /process_pdf/);
|
||||
assert.throws(() => classifyPdf(Buffer.from('')), /classify_pdf/);
|
||||
await assert.rejects(processPdfAsync(Buffer.from('not a pdf')), /process_pdf/);
|
||||
await assert.rejects(classifyPdfAsync(Buffer.from('')), /classify_pdf/);
|
||||
await assert.rejects(extractPagesMarkdownAsync(Buffer.from('')), /extract_pages_markdown/);
|
||||
console.log(' error handling: OK');
|
||||
|
||||
console.log('\nAll NAPI tests passed!');
|
||||
|
||||
Reference in New Issue
Block a user