Skip to content

Commit 6381616

Browse files
committed
fix: propagate batchFn sync throws to the loader instead of crashing
1 parent 015a94c commit 6381616

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

src/__tests__/abuse.test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,26 @@ describe('Provides descriptive error messages for API abuse', () => {
9797
);
9898
});
9999

100+
it('Batch function must return a Promise, not error synchronously', async () => {
101+
// $FlowExpectError
102+
const badLoader = new DataLoader<number, number>(() => {
103+
throw new Error("Mock Synchronous Error")
104+
});
105+
106+
let caughtError;
107+
try {
108+
await badLoader.load(1);
109+
} catch (error) {
110+
caughtError = error;
111+
}
112+
expect(caughtError).toBeInstanceOf(Error);
113+
expect((caughtError: any).message).toBe(
114+
'DataLoader must be constructed with a function which accepts ' +
115+
'Array<key> and returns Promise<Array<value>>, but the function ' +
116+
`errored synchronously: Error: Mock Synchronous Error.`
117+
);
118+
});
119+
100120
it('Batch function must return a Promise, not a value', async () => {
101121
// Note: this is returning the keys directly, rather than a promise to keys.
102122
// $FlowExpectError

src/index.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,16 @@ function dispatchBatch<K, V>(
304304

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

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

0 commit comments

Comments
 (0)