Skip to content

Commit 588a8b6

Browse files
authored
fix: propagate batchFn sync throws to the loader instead of crashing (#318)
* fix: propagate batchFn sync throws to the loader instead of crashing * add changeset
1 parent 420573b commit 588a8b6

File tree

3 files changed

+35
-1
lines changed

3 files changed

+35
-1
lines changed

.changeset/lovely-grapes-bake.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"dataloader": patch
3+
---
4+
5+
Fix the propagation of sync throws in the batch function to the loader function instead of crashing the process wtih an uncaught exception.

src/__tests__/abuse.test.js

+20
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

+10-1
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,16 @@ function dispatchBatch<K, V>(
302302

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

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

0 commit comments

Comments
 (0)