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
  • Loading branch information
boopathi committed Oct 21, 2022
1 parent 015a94c commit 6381616
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
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 @@ -304,7 +304,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 6381616

Please sign in to comment.