From 9e54f55c226b83b603cb6e11a295ec62b2308696 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Mon, 11 Apr 2022 10:53:57 -0700 Subject: [PATCH] Ensure that waitForAll functions catch and log thrown errors --- app/main.ts | 7 +++++-- ts/background.ts | 1 + ts/sql/Client.ts | 2 ++ ts/textsecure/MessageReceiver.ts | 1 + ts/util/batcher.ts | 11 ++++++++++- ts/util/waitBatcher.ts | 19 +++++++++++++++++-- 6 files changed, 36 insertions(+), 5 deletions(-) diff --git a/app/main.ts b/app/main.ts index 2474c00ef..1c9e91d7d 100644 --- a/app/main.ts +++ b/app/main.ts @@ -1693,7 +1693,7 @@ async function requestShutdown() { } getLogger().info('requestShutdown: Requesting close of mainWindow...'); - const request = new Promise((resolveFn, reject) => { + const request = new Promise(resolveFn => { let timeout: NodeJS.Timeout | undefined; if (!mainWindow) { @@ -1705,7 +1705,10 @@ async function requestShutdown() { getLogger().info('requestShutdown: Response received'); if (error) { - return reject(error); + getLogger().error( + 'requestShutdown: got error, still shutting down.', + error + ); } clearTimeoutIfNecessary(timeout); diff --git a/ts/background.ts b/ts/background.ts index 82171da5b..ec59cd105 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -648,6 +648,7 @@ export async function startApp(): Promise { server !== undefined, 'WebAPI should be initialized together with MessageReceiver' ); + log.info('background/shutdown: shutting down messageReceiver'); server.unregisterRequestHandler(messageReceiver); messageReceiver.stopProcessing(); await window.waitForAllBatchers(); diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index 42c2e0de4..c320d3b03 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -673,6 +673,8 @@ function keysFromBytes(keys: Array, data: any) { // Top-level calls async function shutdown() { + log.info('Client.shutdown'); + // Stop accepting new SQL jobs, flush outstanding queue await _shutdown(); diff --git a/ts/textsecure/MessageReceiver.ts b/ts/textsecure/MessageReceiver.ts index 021969a67..0a08ce3eb 100644 --- a/ts/textsecure/MessageReceiver.ts +++ b/ts/textsecure/MessageReceiver.ts @@ -352,6 +352,7 @@ export default class MessageReceiver } public stopProcessing(): void { + log.info('MessageReceiver.stopProcessing'); this.stoppingProcessing = true; } diff --git a/ts/util/batcher.ts b/ts/util/batcher.ts index cc7e37dea..d17785962 100644 --- a/ts/util/batcher.ts +++ b/ts/util/batcher.ts @@ -5,6 +5,7 @@ import PQueue from 'p-queue'; import { sleep } from './sleep'; import * as log from '../logging/log'; +import * as Errors from '../types/errors'; import { clearTimeoutIfNecessary } from './clearTimeoutIfNecessary'; declare global { @@ -20,7 +21,15 @@ declare global { window.batchers = []; window.waitForAllBatchers = async () => { - await Promise.all(window.batchers.map(item => item.flushAndWait())); + log.info('batcher#waitForAllBatchers'); + try { + await Promise.all(window.batchers.map(item => item.flushAndWait())); + } catch (error) { + log.error( + 'waitForAllBatchers: error flushing all', + Errors.toLogFormat(error) + ); + } }; export type BatcherOptionsType = { diff --git a/ts/util/waitBatcher.ts b/ts/util/waitBatcher.ts index 9d9cfd213..25acf2a76 100644 --- a/ts/util/waitBatcher.ts +++ b/ts/util/waitBatcher.ts @@ -5,6 +5,7 @@ import PQueue from 'p-queue'; import { sleep } from './sleep'; import * as log from '../logging/log'; +import * as Errors from '../types/errors'; import { clearTimeoutIfNecessary } from './clearTimeoutIfNecessary'; declare global { @@ -22,12 +23,26 @@ window.waitBatchers = []; window.flushAllWaitBatchers = async () => { log.info('waitBatcher#flushAllWaitBatchers'); - await Promise.all(window.waitBatchers.map(item => item.flushAndWait())); + try { + await Promise.all(window.waitBatchers.map(item => item.flushAndWait())); + } catch (error) { + log.error( + 'flushAllWaitBatchers: Error flushing all', + Errors.toLogFormat(error) + ); + } }; window.waitForAllWaitBatchers = async () => { log.info('waitBatcher#waitForAllWaitBatchers'); - await Promise.all(window.waitBatchers.map(item => item.onIdle())); + try { + await Promise.all(window.waitBatchers.map(item => item.onIdle())); + } catch (error) { + log.error( + 'waitForAllWaitBatchers: Error waiting for all', + Errors.toLogFormat(error) + ); + } }; type ItemHolderType = {