From 6d72f8abe7b4ff47f8884fc01b77365c26002327 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Sat, 14 Dec 2024 15:55:09 +0100 Subject: [PATCH] feat: Add FCM option `resolveUnhandledClientError` to resolve or reject push promise on FCM server response GOAWAY (#341) --- .gitignore | 1 + README.md | 10 +++++++ spec/.eslintrc.json | 12 +++++++++ spec/FCM.spec.js | 65 ++++++++++++++++++++++++++++++++++++--------- src/FCM.js | 30 ++++++++++++++++----- 5 files changed, 99 insertions(+), 19 deletions(-) create mode 100644 spec/.eslintrc.json diff --git a/.gitignore b/.gitignore index e06fabc..907d611 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,4 @@ node_modules # Optional eslint cache .eslintcache +.vscode/launch.json diff --git a/README.md b/README.md index 964f9d6..051f683 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ The official Push Notification adapter for Parse Server. See [Parse Server Push - [Google Cloud Service Account Key](#google-cloud-service-account-key) - [Migration to FCM HTTP v1 API (June 2024)](#migration-to-fcm-http-v1-api-june-2024) - [HTTP/1.1 Legacy Option](#http11-legacy-option) + - [Firebase Client Error](#firebase-client-error) - [Expo Push Options](#expo-push-options) - [Bundled with Parse Server](#bundled-with-parse-server) - [Logging](#logging) @@ -158,6 +159,15 @@ android: { } ``` +#### Firebase Client Error + +Occasionally, errors within the Firebase Cloud Messaging (FCM) client may not be managed internally and are instead passed to the Parse Server Push Adapter. These errors can occur, for instance, due to unhandled FCM server connection issues. + +- `resolveUnhandledClientError: true`: Logs the error and gracefully resolves it, ensuring that push sending does not result in a failure. +- `resolveUnhandledClientError: false`: Causes push sending to fail, returning a `Parse.Error.OTHER_CAUSE` with error details that can be parsed to handle it accordingly. This is the default. + +In both cases, detailed error logs are recorded in the Parse Server logs for debugging purposes. + ### Expo Push Options Example options: diff --git a/spec/.eslintrc.json b/spec/.eslintrc.json new file mode 100644 index 0000000..44ad90c --- /dev/null +++ b/spec/.eslintrc.json @@ -0,0 +1,12 @@ +{ + "env": { + "jasmine": true + }, + "globals": { + "Parse": true + }, + "rules": { + "no-console": [0], + "no-var": "error" + } +} diff --git a/spec/FCM.spec.js b/spec/FCM.spec.js index f86102d..2e8f269 100644 --- a/spec/FCM.spec.js +++ b/spec/FCM.spec.js @@ -1,21 +1,24 @@ -import path from 'path'; +import { deleteApp, getApps } from 'firebase-admin/app'; import log from 'npmlog'; +import Parse from 'parse/node.js'; +import path from 'path'; import FCM from '../src/FCM.js'; -import { getApps, deleteApp } from 'firebase-admin/app'; - -const testArgs = { - firebaseServiceAccount: path.join( - __dirname, - '..', - 'spec', - 'support', - 'fakeServiceAccount.json', - ), -}; + +let testArgs; describe('FCM', () => { beforeEach(async () => { getApps().forEach(app => deleteApp(app)); + + testArgs = { + firebaseServiceAccount: path.join( + __dirname, + '..', + 'spec', + 'support', + 'fakeServiceAccount.json', + ), + }; }); it('can initialize', () => { @@ -221,6 +224,44 @@ describe('FCM', () => { expect(spyError).toHaveBeenCalledWith('parse-server-push-adapter FCM', 'error sending push: testing error abort'); }); + it('rejects exceptions that are unhandled by FCM client', async () => { + const spyInfo = spyOn(log, 'info').and.callFake(() => {}); + const spyError = spyOn(log, 'error').and.callFake(() => {}); + testArgs.resolveUnhandledClientError = false; + const fcm = new FCM(testArgs); + spyOn(fcm.sender, 'sendEachForMulticast').and.callFake(() => { + throw new Error('test error'); + }); + fcm.pushType = 'android'; + const data = { data: { alert: 'alert' } }; + const devices = [{ deviceToken: 'token' }]; + await expectAsync(fcm.send(data, devices)).toBeRejectedWith(new Parse.Error(Parse.Error.OTHER_CAUSE, 'Error: test error')); + expect(fcm.sender.sendEachForMulticast).toHaveBeenCalled(); + expect(spyInfo).toHaveBeenCalledWith('parse-server-push-adapter FCM', 'sending push to 1 devices'); + expect(spyError).toHaveBeenCalledTimes(2); + expect(spyError.calls.all()[0].args).toEqual(['parse-server-push-adapter FCM', 'error sending push: firebase client exception: Error: test error']); + expect(spyError.calls.all()[1].args).toEqual(['parse-server-push-adapter FCM', 'error sending push: ParseError: -1 Error: test error']); + }); + + it('resolves exceptions that are unhandled by FCM client', async () => { + const spyInfo = spyOn(log, 'info').and.callFake(() => {}); + const spyError = spyOn(log, 'error').and.callFake(() => {}); + testArgs.resolveUnhandledClientError = true; + const fcm = new FCM(testArgs); + spyOn(fcm.sender, 'sendEachForMulticast').and.callFake(() => { + throw new Error('test error'); + }); + fcm.pushType = 'android'; + const data = { data: { alert: 'alert' } }; + const devices = [{ deviceToken: 'token' }]; + await expectAsync(fcm.send(data, devices)).toBeResolved(); + expect(fcm.sender.sendEachForMulticast).toHaveBeenCalled(); + expect(spyInfo).toHaveBeenCalledWith('parse-server-push-adapter FCM', 'sending push to 1 devices'); + expect(spyError).toHaveBeenCalledTimes(2); + expect(spyError.calls.all()[0].args).toEqual(['parse-server-push-adapter FCM', 'error sending push: firebase client exception: Error: test error']); + expect(spyError.calls.all()[1].args).toEqual(['parse-server-push-adapter FCM', 'error sending push: ParseError: -1 Error: test error']); + }); + it('FCM request invalid push type', async () => { const fcm = new FCM(testArgs); fcm.pushType = 'invalid'; diff --git a/src/FCM.js b/src/FCM.js index b6fc9b1..8634956 100644 --- a/src/FCM.js +++ b/src/FCM.js @@ -1,9 +1,9 @@ 'use strict'; -import Parse from 'parse'; -import log from 'npmlog'; -import { initializeApp, cert, getApps, getApp } from 'firebase-admin/app'; +import { cert, getApp, getApps, initializeApp } from 'firebase-admin/app'; import { getMessaging } from 'firebase-admin/messaging'; +import log from 'npmlog'; +import Parse from 'parse'; import { randomString } from './PushAdapterUtils.js'; const LOG_PREFIX = 'parse-server-push-adapter FCM'; @@ -28,6 +28,9 @@ export default function FCM(args, pushType) { const fcmEnableLegacyHttpTransport = typeof args.fcmEnableLegacyHttpTransport === 'boolean' ? args.fcmEnableLegacyHttpTransport : false; + this.resolveUnhandledClientError = typeof args.resolveUnhandledClientError === 'boolean' + ? args.resolveUnhandledClientError + : false; let app; if (getApps().length === 0) { @@ -88,8 +91,18 @@ FCM.prototype.send = function (data, devices) { const length = deviceTokens.length; log.info(LOG_PREFIX, `sending push to ${length} devices`); - return this.sender - .sendEachForMulticast(fcmPayload.data) + // This is a safe wrapper for sendEachForMulticast, due to bug in the firebase-admin + // library, where it throws an exception instead of returning a rejected promise + const sendEachForMulticastSafe = fcmPayloadData => { + try { + return this.sender.sendEachForMulticast(fcmPayloadData); + } catch (err) { + log.error(LOG_PREFIX, `error sending push: firebase client exception: ${err}`); + return Promise.reject(new Parse.Error(Parse.Error.OTHER_CAUSE, err)); + } + }; + + return sendEachForMulticastSafe(fcmPayload.data) .then((response) => { const promises = []; const failedTokens = []; @@ -140,8 +153,11 @@ FCM.prototype.send = function (data, devices) { const allPromises = Promise.all( slices.map((slice) => sendToDeviceSlice(slice, this.pushType)), - ).catch((err) => { - log.error(LOG_PREFIX, `error sending push: ${err}`); + ).catch(e => { + log.error(LOG_PREFIX, `error sending push: ${e}`); + if (!this.resolveUnhandledClientError && e instanceof Parse.Error && e.code === Parse.Error.OTHER_CAUSE) { + return Promise.reject(e); + } }); return allPromises;