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 931eb51
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 4 deletions.
32 changes: 32 additions & 0 deletions src/__tests__/dataloader.test.js
Expand Up @@ -341,6 +341,9 @@ describe('Represents Errors', () => {

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

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

let caughtErrorA;
try {
await identityLoader.load(1);
Expand All @@ -353,6 +356,35 @@ describe('Represents Errors', () => {
expect(loadCalls).toEqual([]);
});

// TODO: #224
/*
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('Can clear values from cache after errors', async () => {
const loadCalls = [];
const errorLoader = new DataLoader(keys => {
Expand Down
13 changes: 9 additions & 4 deletions src/index.js
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 931eb51

Please sign in to comment.