Skip to content

Commit

Permalink
[keyserver] Replace deprecated sendToDevice firebase-admin method
Browse files Browse the repository at this point in the history
Summary: This method is deprecated and is now completely failing all requests. This diff resolves [ENG-235](https://linear.app/comm/issue/ENG-235/update-fcm-code-to-not-use-legacy-methods).

Test Plan:
I set up a test environment on my personal server. It's running the latest keyserver release (115). In this environment, I created a [test script](https://linear.app/comm/issue/ENG-9323/android-notifications-are-completely-broken#comment-516bbedd) to generate some notifs by sending messages from commbot to a test account. I confirmed that the test account received a notif on an Android device it's logged into.

I also did some testing to make sure the error handling logic worked correctly. In my test environment, I set the device token to `fake` and checked what the error response looked like, and made sure that `error.errorInfo.code` contained the error code. For my fake device token, the error code ended up being `messaging/invalid-argument`, but I'm pretty sure that the error code would appear in the same place for the types of errors we care about (`fcmTokenInvalidationErrors`).

Reviewers: varun, will

Reviewed By: varun, will

Subscribers: marcin, kamil, tomek

Differential Revision: https://phab.comm.dev/D13368
  • Loading branch information
Ashoat committed Sep 17, 2024
1 parent 849adf8 commit 3733bf3
Show file tree
Hide file tree
Showing 4 changed files with 370 additions and 328 deletions.
6 changes: 1 addition & 5 deletions keyserver/flow-typed/npm/firebase-admin_vx.x.x.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@ declare module 'firebase-admin' {
};

declare export type FirebaseMessaging = {
+sendToDevice: (
deviceToken: string,
notification: Object,
options: Object,
) => Promise<FirebaseDeliveryResult>,
+send: (notif: Object) => Promise<string>,
...
};

Expand Down
2 changes: 1 addition & 1 deletion keyserver/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"express": "^4.17.3",
"express-ws": "^4.0.0",
"file-type": "^12.3.0",
"firebase-admin": "^10.1.0",
"firebase-admin": "^12.5.0",
"geoip-lite": "^1.4.5",
"invariant": "^2.2.4",
"landing": "0.0.1",
Expand Down
44 changes: 19 additions & 25 deletions keyserver/src/push/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,16 @@ async function fcmPush({
for (let i = 0; i < results.length; i++) {
const pushResult = results[i];
for (const error of pushResult.errors) {
errors.push(error.error);
const errorCode =
error.type === 'firebase_error'
? error.error.errorInfo.code
: undefined;
if (errorCode && fcmTokenInvalidationErrors.has(errorCode)) {
errors.push(error);
if (
error &&
typeof error === 'object' &&
error.errorInfo &&
typeof error.errorInfo === 'object' &&
error.errorInfo.code &&
typeof error.errorInfo.code === 'string' &&
fcmTokenInvalidationErrors.has(error.errorInfo.code)
) {
invalidTokens.push(targetedNotifications[i].deliveryID);
}
}
Expand All @@ -171,12 +175,9 @@ async function fcmPush({
return result;
}

type FCMSinglePushError =
| { +type: 'firebase_error', +error: FirebaseError }
| { +type: 'exception', +error: mixed };
type FCMSinglePushResult = {
+fcmIDs: $ReadOnlyArray<string>,
+errors: $ReadOnlyArray<FCMSinglePushError>,
+errors: $ReadOnlyArray<mixed>,
};
async function fcmSinglePush(
provider: FirebaseApp,
Expand All @@ -185,21 +186,14 @@ async function fcmSinglePush(
options: Object,
): Promise<FCMSinglePushResult> {
try {
const deliveryResult = await provider
.messaging()
.sendToDevice(deviceToken, notification, options);
const errors = [];
const ids = [];
for (const fcmResult of deliveryResult.results) {
if (fcmResult.error) {
errors.push({ type: 'firebase_error', error: fcmResult.error });
} else if (fcmResult.messageId) {
ids.push(fcmResult.messageId);
}
}
return { fcmIDs: ids, errors };
const fcmMessageID = await provider.messaging().send({
...notification,
token: deviceToken,
android: options,
});
return { fcmIDs: [fcmMessageID], errors: [] };
} catch (e) {
return { fcmIDs: [], errors: [{ type: 'exception', error: e }] };
return { fcmIDs: [], errors: [e] };
}
}

Expand All @@ -211,7 +205,7 @@ async function getUnreadCounts(
const query = SQL`
SELECT user, COUNT(thread) AS unread_count
FROM memberships
WHERE user IN (${userIDs}) AND last_message > last_read_message
WHERE user IN (${userIDs}) AND last_message > last_read_message
AND role > 0
AND JSON_EXTRACT(permissions, ${visPermissionExtractString})
AND JSON_EXTRACT(subscription, ${notificationExtractString})
Expand Down
Loading

0 comments on commit 3733bf3

Please sign in to comment.