Skip to content

Commit

Permalink
fix: propagate batchFn sync throws to the loader instead of crashing (#…
Browse files Browse the repository at this point in the history
…318)

* fix: propagate batchFn sync throws to the loader instead of crashing

* add changeset
  • Loading branch information
boopathi committed Jan 13, 2023
1 parent 420573b commit 588a8b6
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/lovely-grapes-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"dataloader": patch
---

Fix the propagation of sync throws in the batch function to the loader function instead of crashing the process wtih an uncaught exception.
20 changes: 20 additions & 0 deletions src/__tests__/abuse.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,26 @@ describe('Provides descriptive error messages for API abuse', () => {
);
});

it('Batch function must return a Promise, not error synchronously', async () => {
// $FlowExpectError
const badLoader = new DataLoader<number, number>(() => {
throw new Error("Mock Synchronous Error")
});

let caughtError;
try {
await badLoader.load(1);
} catch (error) {
caughtError = error;
}
expect(caughtError).toBeInstanceOf(Error);
expect((caughtError: any).message).toBe(
'DataLoader must be constructed with a function which accepts ' +
'Array<key> and returns Promise<Array<value>>, but the function ' +
`errored synchronously: Error: Mock Synchronous Error.`
);
});

it('Batch function must return a Promise, not a value', async () => {
// Note: this is returning the keys directly, rather than a promise to keys.
// $FlowExpectError
Expand Down
11 changes: 10 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,16 @@ function dispatchBatch<K, V>(

// Call the provided batchLoadFn for this loader with the batch's keys and
// with the loader as the `this` context.
var batchPromise = loader._batchLoadFn(batch.keys);
var batchPromise;
try {
batchPromise = loader._batchLoadFn(batch.keys);
} catch (e) {
return failedDispatch(loader, batch, new TypeError(
'DataLoader must be constructed with a function which accepts ' +
'Array<key> and returns Promise<Array<value>>, but the function ' +
`errored synchronously: ${String(e)}.`
));
}

// Assert the expected response from batchLoadFn
if (!batchPromise || typeof batchPromise.then !== 'function') {
Expand Down

0 comments on commit 588a8b6

Please sign in to comment.