From 931eb51fd27c35f65f327a8b18adb114e402aa44 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Thu, 14 Nov 2019 13:47:34 -0800 Subject: [PATCH] [FIX] Fix case where priming a cache with an Error results in UnhandledPromiseRejection --- src/__tests__/dataloader.test.js | 32 ++++++++++++++++++++++++++++++++ src/index.js | 13 +++++++++---- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/__tests__/dataloader.test.js b/src/__tests__/dataloader.test.js index aebf5f7..1500ef1 100644 --- a/src/__tests__/dataloader.test.js +++ b/src/__tests__/dataloader.test.js @@ -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); @@ -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(); + + 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 => { diff --git a/src/index.js b/src/index.js index f109d48..3068ac4 100644 --- a/src/index.js +++ b/src/index.js @@ -184,10 +184,15 @@ class DataLoader { 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); } }