From a913911592ea044a50aed2f324a24db4e7b8dea1 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Wed, 19 Aug 2020 09:53:31 -0400 Subject: [PATCH] Use async fn's .catch to avoid unhandled rejection Add explicit `isPending` value instead of overloading role of `status`. Could probably do without it, but it makes the intent more clear. --- .../server/services/setup_utils.test.ts | 20 +++++++++--------- .../server/services/setup_utils.ts | 21 ++++++++----------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/ingest_manager/server/services/setup_utils.test.ts b/x-pack/plugins/ingest_manager/server/services/setup_utils.test.ts index f1fab2401330d4..8d71fc48a21291 100644 --- a/x-pack/plugins/ingest_manager/server/services/setup_utils.test.ts +++ b/x-pack/plugins/ingest_manager/server/services/setup_utils.test.ts @@ -12,10 +12,10 @@ async function sleep(ms: number) { describe('awaitIfPending', () => { it('first promise called blocks others', async () => { - const fnA = jest.fn(); - const fnB = jest.fn(); - const fnC = jest.fn(); - const fnD = jest.fn(); + const fnA = jest.fn().mockImplementation(async () => {}); + const fnB = jest.fn().mockImplementation(async () => {}); + const fnC = jest.fn().mockImplementation(async () => {}); + const fnD = jest.fn().mockImplementation(async () => {}); const promises = [ awaitIfPending(fnA), awaitIfPending(fnB), @@ -92,12 +92,12 @@ describe('awaitIfPending', () => { it('does not block other calls after batch is fulfilled. can call again for a new result', async () => { const fnA = jest .fn() - .mockImplementationOnce(() => 'fnA first') - .mockImplementationOnce(() => 'fnA second') - .mockImplementation(() => 'fnA default/2+'); - const fnB = jest.fn(); - const fnC = jest.fn(); - const fnD = jest.fn(); + .mockImplementationOnce(async () => 'fnA first') + .mockImplementationOnce(async () => 'fnA second') + .mockImplementation(async () => 'fnA default/2+'); + const fnB = jest.fn().mockImplementation(async () => {}); + const fnC = jest.fn().mockImplementation(async () => {}); + const fnD = jest.fn().mockImplementation(async () => {}); let promises = [ awaitIfPending(fnA), awaitIfPending(fnB), diff --git a/x-pack/plugins/ingest_manager/server/services/setup_utils.ts b/x-pack/plugins/ingest_manager/server/services/setup_utils.ts index 4632efbcdeb7bb..3c752bd410c5ae 100644 --- a/x-pack/plugins/ingest_manager/server/services/setup_utils.ts +++ b/x-pack/plugins/ingest_manager/server/services/setup_utils.ts @@ -5,36 +5,33 @@ */ // the promise which tracks the setup -let status: Promise | undefined; +let status: Promise | undefined; +let isPending = false; // default resolve to guard against "undefined is not a function" errors let onResolve = (value?: unknown) => {}; let onReject = (reason: any) => {}; -export async function awaitIfPending(asyncFunction: Function) { +export async function awaitIfPending(asyncFunction: Function): Promise { // pending successful or failed attempt - if (status) { + if (isPending) { // don't run concurrent installs + // return a promise which will eventually resolve/reject return status; } else { // create the initial promise status = new Promise((res, rej) => { + isPending = true; onResolve = res; onReject = rej; }); } try { - // if everything works, mark the tracking promise as resolved - const result = await asyncFunction(); + const result = await asyncFunction().catch(onReject); onResolve(result); - // * reset the tracking promise to try again next time - status = undefined; - return result; } catch (error) { // if something fails onReject(error); - // * reset the tracking promise to try again next time - status = undefined; - // * return the rejection so it can be dealt with now - return Promise.reject(error); } + isPending = false; + return status; }