From 964f151fb55bc219ba291dea4a3b441f74a67352 Mon Sep 17 00:00:00 2001 From: Jonathan Ferreira <44679989+Jonathansoufer@users.noreply.github.com> Date: Wed, 25 Sep 2024 19:08:05 +0100 Subject: [PATCH] feat: add timeout handler (#11429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR adds a withTimeout wrapper on notifee async calls to avoid get blocked in case a call doesn't resolve quickly. This was a comment from [this PR](https://github.com/MetaMask/metamask-mobile/pull/11250#discussion_r1773684001) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- app/util/notifications/methods/common.ts | 7 +++++++ .../services/NotificationService.test.ts | 12 ++++++++---- .../services/NotificationService.ts | 18 +++++++++++------- locales/languages/en.json | 6 +++--- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/app/util/notifications/methods/common.ts b/app/util/notifications/methods/common.ts index c7ac73eeacd..171d150f517 100644 --- a/app/util/notifications/methods/common.ts +++ b/app/util/notifications/methods/common.ts @@ -474,3 +474,10 @@ export const getUsdAmount = (amount: string, decimals: string, usd: string) => { export const hasInitialNotification = async () => Boolean(await notifee.getInitialNotification()); + +export function withTimeout(promise: Promise, ms: number): Promise { + const timeout = new Promise((_, reject) => + setTimeout(() => reject(new Error(strings('notifications.timeout'))), ms), + ); + return Promise.race([promise, timeout]); +} diff --git a/app/util/notifications/services/NotificationService.test.ts b/app/util/notifications/services/NotificationService.test.ts index 9816f9f99f4..e40e78dd47e 100644 --- a/app/util/notifications/services/NotificationService.test.ts +++ b/app/util/notifications/services/NotificationService.test.ts @@ -83,10 +83,14 @@ describe('NotificationsService', () => { expect(notifee.createChannel).toHaveBeenCalledWith(channel); }); - it('should return authorized from getAllPermissions', async () => { - const result = await NotificationsService.getAllPermissions(); - expect(result.permission).toBe('authorized'); - }); + it.concurrent( + 'should return authorized from getAllPermissions', + async () => { + const result = await NotificationsService.getAllPermissions(); + expect(result.permission).toBe('authorized'); + }, + 10000, + ); it('should return authorized from requestPermission ', async () => { const result = await NotificationsService.requestPermission(); diff --git a/app/util/notifications/services/NotificationService.ts b/app/util/notifications/services/NotificationService.ts index 782de0dec1a..40b72e0a48c 100644 --- a/app/util/notifications/services/NotificationService.ts +++ b/app/util/notifications/services/NotificationService.ts @@ -19,6 +19,7 @@ import { mmStorage } from '../settings'; import { STORAGE_IDS } from '../settings/storage/constants'; import { store } from '../../../store'; import Logger from '../../../util/Logger'; +import { withTimeout } from '../methods'; interface AlertButton { text: string; @@ -56,18 +57,21 @@ class NotificationsService { } async getAllPermissions(shouldOpenSettings = true) { - const promises = [] as Promise[]; - notificationChannels.forEach((channel: AndroidChannel) => { - promises.push(this.createChannel(channel)); - }); + const promises: Promise[] = notificationChannels.map( + (channel: AndroidChannel) => + withTimeout(this.createChannel(channel), 5000), + ); await Promise.allSettled(promises); - const permission = await this.requestPermission(); - const blockedNotifications = await this.getBlockedNotifications(); + const permission = await withTimeout(this.requestPermission(), 5000); + const blockedNotifications = await withTimeout( + this.getBlockedNotifications(), + 5000, + ); if ( (permission !== 'authorized' || blockedNotifications.size !== 0) && shouldOpenSettings ) { - this.requestPushNotificationsPermission(); + await this.requestPushNotificationsPermission(); } return { permission, blockedNotifications }; } diff --git a/locales/languages/en.json b/locales/languages/en.json index 71d0ac0b1ef..faf48beb6b5 100644 --- a/locales/languages/en.json +++ b/locales/languages/en.json @@ -2077,6 +2077,7 @@ "back_to_safety": "Back to safety" }, "notifications": { + "timeout": "Timeout", "no_date": "Unknown", "yesterday": "Yesterday", "staked": "Staked", @@ -3230,7 +3231,6 @@ "ledger_legacy_label": " (legacy)", "blind_sign_error": "Blind signing error", "blind_sign_error_message": "Blind signing is not enabled on your Ledger device. Please enable it in the settings." - }, "account_actions": { "edit_name": "Edit account name", @@ -3298,7 +3298,7 @@ "enter_amount": "Enter amount", "review": "Review", "not_enough_eth": "Not enough ETH", - "balance":"Balance" + "balance": "Balance" }, "default_settings": { "title": "Your Wallet is ready", @@ -3357,4 +3357,4 @@ "tooltip": "Expected yearly increase in the value of your stake, based on the reward rate over the past week." } } -} +} \ No newline at end of file