From a426e1e41495086da0ddb7c380fe4d22a47c9e2e 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 | 52 ++++++++++++++++++++++++++++++++ src/index.js | 13 +++++--- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/__tests__/dataloader.test.js b/src/__tests__/dataloader.test.js index aebf5f7..1b7be14 100644 --- a/src/__tests__/dataloader.test.js +++ b/src/__tests__/dataloader.test.js @@ -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(); + + 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(); + + identityLoader.prime(1, new Error('Error: 1')); + + // Wait a bit. + await new Promise(setImmediate); + let caughtErrorA; try { await identityLoader.load(1); 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); } }