Skip to content

Commit

Permalink
[FIX] Fix case where priming a cache with an Error results in Unhandl…
Browse files Browse the repository at this point in the history
…edPromiseRejection
  • Loading branch information
leebyron committed Nov 14, 2019
1 parent 4212c9e commit a426e1e
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 4 deletions.
52 changes: 52 additions & 0 deletions src/__tests__/dataloader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,58 @@ describe('Represents Errors', () => {

identityLoader.prime(1, new Error('Error: 1'));

// Wait a bit.
await new Promise(setImmediate);

let caughtErrorA;
try {
await identityLoader.load(1);
} catch (error) {
caughtErrorA = error;
}
expect(caughtErrorA).toBeInstanceOf(Error);
expect((caughtErrorA: any).message).toBe('Error: 1');

expect(loadCalls).toEqual([]);
});

// TODO: this is a minor behavioral bug.
/*
it('Not catching a primed error is an unhandled rejection', async () => {
let hadUnhandledRejection = false;
function onUnhandledRejection() {
hadUnhandledRejection = true;
}
process.on('unhandledRejection', onUnhandledRejection);
try {
const [ identityLoader ] = idLoader<number>();
identityLoader.prime(1, new Error('Error: 1'));
// Wait a bit.
await new Promise(setImmediate);
// Ignore result.
identityLoader.load(1);
// Wait a bit.
await new Promise(setImmediate);
expect(hadUnhandledRejection).toBe(true);
} finally {
process.removeListener('unhandledRejection', onUnhandledRejection);
}
});
*/

it('Handles priming the cache with an error', async () => {
const [ identityLoader, loadCalls ] = idLoader<number>();

identityLoader.prime(1, new Error('Error: 1'));

// Wait a bit.
await new Promise(setImmediate);

let caughtErrorA;
try {
await identityLoader.load(1);
Expand Down
13 changes: 9 additions & 4 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,15 @@ class DataLoader<K, V, C = K> {
if (cache.get(cacheKey) === undefined) {
// Cache a rejected promise if the value is an Error, in order to match
// the behavior of load(key).
var promise = value instanceof Error ?
Promise.reject(value) :
Promise.resolve(value);

var promise;
if (value instanceof Error) {
promise = Promise.reject(value);
// Since this is a case where an Error is intentionally being primed
// for a given key, we want to disable unhandled promise rejection.
promise.catch(() => {});
} else {
promise = Promise.resolve(value);
}
cache.set(cacheKey, promise);
}
}
Expand Down

0 comments on commit a426e1e

Please sign in to comment.