From f41750077a109162a760606d741f9ff910cf890c Mon Sep 17 00:00:00 2001 From: Ryan Welish Date: Fri, 12 Jul 2024 14:43:32 -0400 Subject: [PATCH 1/5] Retry topic subscription operations on quota errors using exponential backoff --- .../com/google/firebase/messaging/GmsRpc.java | 99 ++++++++++++++----- .../firebase/messaging/TopicsSubscriber.java | 3 +- 2 files changed, 78 insertions(+), 24 deletions(-) diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java index e3849e279de..131d7de0872 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java @@ -35,17 +35,26 @@ import java.security.NoSuchAlgorithmException; import java.util.concurrent.ExecutionException; -/** Rpc based on Google Play Service. */ +/** + * Rpc based on Google Play Service. + */ class GmsRpc { + static final String TAG = FirebaseMessaging.TAG; - /** Normal response from GMS */ + /** + * Normal response from GMS + */ private static final String EXTRA_REGISTRATION_ID = "registration_id"; - /** Extra used to indicate that the application has been unregistered. */ + /** + * Extra used to indicate that the application has been unregistered. + */ private static final String EXTRA_UNREGISTERED = "unregistered"; - /** Returned by GMS in case of error. */ + /** + * Returned by GMS in case of error. + */ private static final String EXTRA_ERROR = "error"; /** @@ -55,10 +64,20 @@ class GmsRpc { */ static final String ERROR_SERVICE_NOT_AVAILABLE = "SERVICE_NOT_AVAILABLE"; - /** Another server error besides ERROR_SERVICE_NOT_AVAILABLE that we retry on. */ + /** + * Another server error besides ERROR_SERVICE_NOT_AVAILABLE that we retry on. + */ static final String ERROR_INTERNAL_SERVER_ERROR = "INTERNAL_SERVER_ERROR"; - /** Heartbeat tag for firebase iid. */ + /** + * A server error that represents hitting topic subscription quota. Trying again here may continue + * to fail, but as long as we use exponential backoff its okay to retry. + */ + static final String TOO_MANY_SUBSCRIBERS = "TOO_MANY_SUBSCRIBERS"; + + /** + * Heartbeat tag for firebase iid. + */ static final String FIREBASE_IID_HEARTBEAT_TAG = "fire-iid"; /** @@ -72,49 +91,79 @@ class GmsRpc { private static final String TOPIC_PREFIX = "/topics/"; // LINT.IfChange - /** InstanceId should be reset. Can be a duplicate, or deleted. */ + /** + * InstanceId should be reset. Can be a duplicate, or deleted. + */ static final String ERROR_INSTANCE_ID_RESET = "INSTANCE_ID_RESET"; // LINT.ThenChange(//depot/google3/firebase/instance_id/client/cpp/src/android/instance_id.cc) // --- List of parameters sent to the /register3 servlet - /** Internal parameter used to indicate a 'subtype'. Will not be stored in DB for Nacho. */ + /** + * Internal parameter used to indicate a 'subtype'. Will not be stored in DB for Nacho. + */ private static final String EXTRA_SUBTYPE = "subtype"; - /** Extra used to indicate which senders (Google API project IDs) can send messages to the app */ + /** + * Extra used to indicate which senders (Google API project IDs) can send messages to the app + */ private static final String EXTRA_SENDER = "sender"; private static final String EXTRA_SCOPE = "scope"; - /** Extra sent to http endpoint to indicate delete request */ + /** + * Extra sent to http endpoint to indicate delete request + */ private static final String EXTRA_DELETE = "delete"; - /** Currently we only support the (gdpr) 'delete' operation */ + /** + * Currently we only support the (gdpr) 'delete' operation + */ private static final String EXTRA_IID_OPERATION = "iid-operation"; - /** key id - sha of public key truncated to 8 bytes, with 0x9 prefix */ + /** + * key id - sha of public key truncated to 8 bytes, with 0x9 prefix + */ private static final String PARAM_INSTANCE_ID = "appid"; - /** key id - user agent string published by firebase-common */ + /** + * key id - user agent string published by firebase-common + */ private static final String PARAM_USER_AGENT = "Firebase-Client"; - /** key id - heartbeat code published by firebase-common */ + /** + * key id - heartbeat code published by firebase-common + */ private static final String PARAM_HEARTBEAT_CODE = "Firebase-Client-Log-Type"; - /** Version of the client library. String like: "fcm-112233" */ + /** + * Version of the client library. String like: "fcm-112233" + */ private static final String PARAM_CLIENT_VER = "cliv"; - /** gmp_app_id (application identifier in firebase). String */ + /** + * gmp_app_id (application identifier in firebase). String + */ private static final String PARAM_GMP_APP_ID = "gmp_app_id"; - /** version of the gms package. Integer.toString() */ + /** + * version of the gms package. Integer.toString() + */ private static final String PARAM_GMS_VER = "gmsv"; - /** android build version. Integer.toString() */ + /** + * android build version. Integer.toString() + */ private static final String PARAM_OS_VER = "osv"; - /** package version code. Integer.toString() */ + /** + * package version code. Integer.toString() + */ private static final String PARAM_APP_VER_CODE = "app_ver"; - /** package version name. Integer.toString() */ + /** + * package version name. Integer.toString() + */ private static final String PARAM_APP_VER_NAME = "app_ver_name"; private static final String PARAM_FIS_AUTH_TOKEN = "Goog-Firebase-Installations-Auth"; - /** hashed value of developer chosen (nick)name of Firebase Core SDK (a.k.a. FirebaseApp) */ + /** + * hashed value of developer chosen (nick)name of Firebase Core SDK (a.k.a. FirebaseApp) + */ private static final String PARAM_FIREBASE_APP_NAME_HASH = "firebase-app-name-hash"; // --- End of the params for /register3 @@ -126,10 +175,14 @@ class GmsRpc { */ static final String CMD_RST_FULL = "RST_FULL"; - /** Value included in a GCM message from IID, indicating an identity reset. */ + /** + * Value included in a GCM message from IID, indicating an identity reset. + */ static final String CMD_RST = "RST"; - /** Value included in a GCM message from IID, indicating a token sync reset. */ + /** + * Value included in a GCM message from IID, indicating a token sync reset. + */ static final String CMD_SYNC = "SYNC"; private static final String SCOPE_ALL = "*"; diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java index ca532dcd684..e4dd58280a2 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java @@ -251,7 +251,8 @@ boolean performTopicOperation(TopicOperation topicOperation) throws IOException } catch (IOException e) { // Operation failed, retry failed only if errors from backend are server related error if (GmsRpc.ERROR_SERVICE_NOT_AVAILABLE.equals(e.getMessage()) - || GmsRpc.ERROR_INTERNAL_SERVER_ERROR.equals(e.getMessage())) { + || GmsRpc.ERROR_INTERNAL_SERVER_ERROR.equals(e.getMessage()) + || GmsRpc.TOO_MANY_SUBSCRIBERS.equals(e.getMessage())) { Log.e(TAG, "Topic operation failed: " + e.getMessage() + ". Will retry Topic operation."); return false; // will retry From b59f8068ef87db7aed46b25801081800b87beb9e Mon Sep 17 00:00:00 2001 From: Ryan Welish Date: Fri, 12 Jul 2024 15:28:48 -0400 Subject: [PATCH 2/5] Reformat GmsRpc --- .../com/google/firebase/messaging/GmsRpc.java | 98 +++++-------------- 1 file changed, 25 insertions(+), 73 deletions(-) diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java index 131d7de0872..e7946f23751 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java @@ -35,26 +35,17 @@ import java.security.NoSuchAlgorithmException; import java.util.concurrent.ExecutionException; -/** - * Rpc based on Google Play Service. - */ +/** Rpc based on Google Play Service. */ class GmsRpc { - static final String TAG = FirebaseMessaging.TAG; - /** - * Normal response from GMS - */ + /** Normal response from GMS */ private static final String EXTRA_REGISTRATION_ID = "registration_id"; - /** - * Extra used to indicate that the application has been unregistered. - */ + /** Extra used to indicate that the application has been unregistered. */ private static final String EXTRA_UNREGISTERED = "unregistered"; - /** - * Returned by GMS in case of error. - */ + /** Returned by GMS in case of error. */ private static final String EXTRA_ERROR = "error"; /** @@ -64,20 +55,15 @@ class GmsRpc { */ static final String ERROR_SERVICE_NOT_AVAILABLE = "SERVICE_NOT_AVAILABLE"; - /** - * Another server error besides ERROR_SERVICE_NOT_AVAILABLE that we retry on. - */ + /** Another server error besides ERROR_SERVICE_NOT_AVAILABLE that we retry on. */ static final String ERROR_INTERNAL_SERVER_ERROR = "INTERNAL_SERVER_ERROR"; - /** - * A server error that represents hitting topic subscription quota. Trying again here may continue - * to fail, but as long as we use exponential backoff its okay to retry. + /** A server error that represents hitting topic subscription quota. Trying again here may + * continue to fail, but as long as we use exponential backoff its okay to retry. */ static final String TOO_MANY_SUBSCRIBERS = "TOO_MANY_SUBSCRIBERS"; - /** - * Heartbeat tag for firebase iid. - */ + /** Heartbeat tag for firebase iid. */ static final String FIREBASE_IID_HEARTBEAT_TAG = "fire-iid"; /** @@ -91,79 +77,49 @@ class GmsRpc { private static final String TOPIC_PREFIX = "/topics/"; // LINT.IfChange - /** - * InstanceId should be reset. Can be a duplicate, or deleted. - */ + /** InstanceId should be reset. Can be a duplicate, or deleted. */ static final String ERROR_INSTANCE_ID_RESET = "INSTANCE_ID_RESET"; // LINT.ThenChange(//depot/google3/firebase/instance_id/client/cpp/src/android/instance_id.cc) // --- List of parameters sent to the /register3 servlet - /** - * Internal parameter used to indicate a 'subtype'. Will not be stored in DB for Nacho. - */ + /** Internal parameter used to indicate a 'subtype'. Will not be stored in DB for Nacho. */ private static final String EXTRA_SUBTYPE = "subtype"; - /** - * Extra used to indicate which senders (Google API project IDs) can send messages to the app - */ + /** Extra used to indicate which senders (Google API project IDs) can send messages to the app */ private static final String EXTRA_SENDER = "sender"; private static final String EXTRA_SCOPE = "scope"; - /** - * Extra sent to http endpoint to indicate delete request - */ + /** Extra sent to http endpoint to indicate delete request */ private static final String EXTRA_DELETE = "delete"; - /** - * Currently we only support the (gdpr) 'delete' operation - */ + /** Currently we only support the (gdpr) 'delete' operation */ private static final String EXTRA_IID_OPERATION = "iid-operation"; - /** - * key id - sha of public key truncated to 8 bytes, with 0x9 prefix - */ + /** key id - sha of public key truncated to 8 bytes, with 0x9 prefix */ private static final String PARAM_INSTANCE_ID = "appid"; - /** - * key id - user agent string published by firebase-common - */ + /** key id - user agent string published by firebase-common */ private static final String PARAM_USER_AGENT = "Firebase-Client"; - /** - * key id - heartbeat code published by firebase-common - */ + /** key id - heartbeat code published by firebase-common */ private static final String PARAM_HEARTBEAT_CODE = "Firebase-Client-Log-Type"; - /** - * Version of the client library. String like: "fcm-112233" - */ + /** Version of the client library. String like: "fcm-112233" */ private static final String PARAM_CLIENT_VER = "cliv"; - /** - * gmp_app_id (application identifier in firebase). String - */ + /** gmp_app_id (application identifier in firebase). String */ private static final String PARAM_GMP_APP_ID = "gmp_app_id"; - /** - * version of the gms package. Integer.toString() - */ + /** version of the gms package. Integer.toString() */ private static final String PARAM_GMS_VER = "gmsv"; - /** - * android build version. Integer.toString() - */ + /** android build version. Integer.toString() */ private static final String PARAM_OS_VER = "osv"; - /** - * package version code. Integer.toString() - */ + /** package version code. Integer.toString() */ private static final String PARAM_APP_VER_CODE = "app_ver"; - /** - * package version name. Integer.toString() - */ + /** package version name. Integer.toString() */ private static final String PARAM_APP_VER_NAME = "app_ver_name"; private static final String PARAM_FIS_AUTH_TOKEN = "Goog-Firebase-Installations-Auth"; - /** - * hashed value of developer chosen (nick)name of Firebase Core SDK (a.k.a. FirebaseApp) - */ + /** hashed value of developer chosen (nick)name of Firebase Core SDK (a.k.a. FirebaseApp) */ private static final String PARAM_FIREBASE_APP_NAME_HASH = "firebase-app-name-hash"; // --- End of the params for /register3 @@ -175,14 +131,10 @@ class GmsRpc { */ static final String CMD_RST_FULL = "RST_FULL"; - /** - * Value included in a GCM message from IID, indicating an identity reset. - */ + /** Value included in a GCM message from IID, indicating an identity reset. */ static final String CMD_RST = "RST"; - /** - * Value included in a GCM message from IID, indicating a token sync reset. - */ + /** Value included in a GCM message from IID, indicating a token sync reset. */ static final String CMD_SYNC = "SYNC"; private static final String SCOPE_ALL = "*"; From f4defa217d3e0ac468bc43dcf0e727ec318cf4a0 Mon Sep 17 00:00:00 2001 From: Ryan Welish Date: Fri, 12 Jul 2024 15:40:58 -0400 Subject: [PATCH 3/5] Add changelog entry --- firebase-messaging/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/firebase-messaging/CHANGELOG.md b/firebase-messaging/CHANGELOG.md index a5a447f5cde..b5636fc4345 100644 --- a/firebase-messaging/CHANGELOG.md +++ b/firebase-messaging/CHANGELOG.md @@ -5,6 +5,8 @@ * [changed] Switched Firelog to use the new TransportBackend. * [changed] Log analytics for notifications displayed by Google Play services on behalf of the app. +* [changed] Retry Topic Subscribe/Unsubscribe operations with exponential + backoff if they hit a quota error. ## Kotlin From e1bdc79725ddbb77a9d8811adafc959c88e91b3e Mon Sep 17 00:00:00 2001 From: Ryan Welish Date: Mon, 15 Jul 2024 10:56:12 -0400 Subject: [PATCH 4/5] Reformat changes --- .../src/main/java/com/google/firebase/messaging/GmsRpc.java | 3 ++- .../java/com/google/firebase/messaging/TopicsSubscriber.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java index e7946f23751..f3dc9e4c929 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/GmsRpc.java @@ -58,7 +58,8 @@ class GmsRpc { /** Another server error besides ERROR_SERVICE_NOT_AVAILABLE that we retry on. */ static final String ERROR_INTERNAL_SERVER_ERROR = "INTERNAL_SERVER_ERROR"; - /** A server error that represents hitting topic subscription quota. Trying again here may + /** + * A server error that represents hitting topic subscription quota. Trying again here may * continue to fail, but as long as we use exponential backoff its okay to retry. */ static final String TOO_MANY_SUBSCRIBERS = "TOO_MANY_SUBSCRIBERS"; diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java index e4dd58280a2..6a5b72290cd 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicsSubscriber.java @@ -252,7 +252,7 @@ boolean performTopicOperation(TopicOperation topicOperation) throws IOException // Operation failed, retry failed only if errors from backend are server related error if (GmsRpc.ERROR_SERVICE_NOT_AVAILABLE.equals(e.getMessage()) || GmsRpc.ERROR_INTERNAL_SERVER_ERROR.equals(e.getMessage()) - || GmsRpc.TOO_MANY_SUBSCRIBERS.equals(e.getMessage())) { + || GmsRpc.TOO_MANY_SUBSCRIBERS.equals(e.getMessage())) { Log.e(TAG, "Topic operation failed: " + e.getMessage() + ". Will retry Topic operation."); return false; // will retry From a1857e0a9560c04b8f7e1bb982daee681a1a6d2a Mon Sep 17 00:00:00 2001 From: Ryan Welish Date: Mon, 15 Jul 2024 10:59:40 -0400 Subject: [PATCH 5/5] fix changelog --- firebase-messaging/CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/firebase-messaging/CHANGELOG.md b/firebase-messaging/CHANGELOG.md index b5636fc4345..eb8b9363394 100644 --- a/firebase-messaging/CHANGELOG.md +++ b/firebase-messaging/CHANGELOG.md @@ -1,12 +1,11 @@ # Unreleased - +* [changed] Retry Topic Subscribe/Unsubscribe operations with exponential + backoff if they hit a quota error. # 24.0.0 * [changed] Switched Firelog to use the new TransportBackend. * [changed] Log analytics for notifications displayed by Google Play services on behalf of the app. -* [changed] Retry Topic Subscribe/Unsubscribe operations with exponential - backoff if they hit a quota error. ## Kotlin