Skip to content

Commit

Permalink
feat: add timeout handler (#11429)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **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](#11250 (comment))

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.
  • Loading branch information
Jonathansoufer authored Sep 25, 2024
1 parent 6036bcb commit 964f151
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 14 deletions.
7 changes: 7 additions & 0 deletions app/util/notifications/methods/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,3 +474,10 @@ export const getUsdAmount = (amount: string, decimals: string, usd: string) => {

export const hasInitialNotification = async () =>
Boolean(await notifee.getInitialNotification());

export function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> {
const timeout = new Promise<never>((_, reject) =>
setTimeout(() => reject(new Error(strings('notifications.timeout'))), ms),
);
return Promise.race([promise, timeout]);
}
12 changes: 8 additions & 4 deletions app/util/notifications/services/NotificationService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
18 changes: 11 additions & 7 deletions app/util/notifications/services/NotificationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -56,18 +57,21 @@ class NotificationsService {
}

async getAllPermissions(shouldOpenSettings = true) {
const promises = [] as Promise<string>[];
notificationChannels.forEach((channel: AndroidChannel) => {
promises.push(this.createChannel(channel));
});
const promises: Promise<string>[] = 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 };
}
Expand Down
6 changes: 3 additions & 3 deletions locales/languages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -2077,6 +2077,7 @@
"back_to_safety": "Back to safety"
},
"notifications": {
"timeout": "Timeout",
"no_date": "Unknown",
"yesterday": "Yesterday",
"staked": "Staked",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -3357,4 +3357,4 @@
"tooltip": "Expected yearly increase in the value of your stake, based on the reward rate over the past week."
}
}
}
}

0 comments on commit 964f151

Please sign in to comment.