From 7611bb4315c43c2eaffae56220bc9acf838a7165 Mon Sep 17 00:00:00 2001 From: Vaibhav Date: Tue, 13 Feb 2024 22:43:01 +0530 Subject: [PATCH 1/3] fix(notifications): handle unrecognized device token in FcmError --- core/notifications/src/executor/fcm/error.rs | 57 +++++++++++++++++++- core/notifications/src/executor/mod.rs | 2 +- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/core/notifications/src/executor/fcm/error.rs b/core/notifications/src/executor/fcm/error.rs index 39bf4af6b8..6d4b86f336 100644 --- a/core/notifications/src/executor/fcm/error.rs +++ b/core/notifications/src/executor/fcm/error.rs @@ -5,5 +5,60 @@ pub enum FcmError { #[error("FcmError - I/O Error: {0}")] IOError(#[from] std::io::Error), #[error("FcmError - GoogleFcm1Error: {0}")] - GoogleFcm1Error(#[from] google_fcm1::Error), + GoogleFcm1Error(google_fcm1::Error), + #[error("FcmError: UnrecognizedDeviceToken: {0}")] + UnrecognizedDeviceToken(google_fcm1::Error), +} + +impl From for FcmError { + fn from(err: google_fcm1::Error) -> Self { + match err { + google_fcm1::Error::BadRequest(ref value) => { + if value["error"]["status"].as_str() == Some("NOT_FOUND") + && value["error"]["details"] + .as_array() + .map_or(false, |details| { + details + .iter() + .any(|detail| detail["errorCode"].as_str() == Some("UNREGISTERED")) + }) + { + return FcmError::UnrecognizedDeviceToken(err); + } + FcmError::GoogleFcm1Error(err) + } + _ => FcmError::GoogleFcm1Error(err), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn unrecognized_device_token_err() { + let err_json = json!({ + "error": { + "code": 404, + "message": "Requested entity was not found.", + "status": "NOT_FOUND", + "details": [ + { + "@type": "type.googleapis.com/google.firebase.fcm.v1.FcmError", + "errorCode": "UNREGISTERED" + } + ] + } + }); + + let err = google_fcm1::Error::BadRequest(err_json); + let converted_err: FcmError = err.into(); + + assert!(matches!( + converted_err, + FcmError::UnrecognizedDeviceToken(_) + )); + } } diff --git a/core/notifications/src/executor/mod.rs b/core/notifications/src/executor/mod.rs index fa37beb76f..18e6295b5c 100644 --- a/core/notifications/src/executor/mod.rs +++ b/core/notifications/src/executor/mod.rs @@ -49,7 +49,7 @@ impl Executor { let mut n_removed_tokens = 0; for device_token in settings.push_device_tokens() { match self.fcm.send(&device_token, &msg, event.deep_link()).await { - Err(FcmError::GoogleFcm1Error(google_fcm1::Error::BadRequest(e))) => { + Err(FcmError::UnrecognizedDeviceToken(e)) => { n_errs += 1; n_removed_tokens += 1; error!("BadRequest sending to device: {}", e); From a186daedc730aa5a2ce8d78d96b2a9d805cb6941 Mon Sep 17 00:00:00 2001 From: Vaibhav Date: Tue, 13 Feb 2024 23:02:28 +0530 Subject: [PATCH 2/3] fix(notifications): validate status code for UnrecognizedDeviceToken error --- core/notifications/src/executor/fcm/error.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/notifications/src/executor/fcm/error.rs b/core/notifications/src/executor/fcm/error.rs index 6d4b86f336..069d808885 100644 --- a/core/notifications/src/executor/fcm/error.rs +++ b/core/notifications/src/executor/fcm/error.rs @@ -14,7 +14,8 @@ impl From for FcmError { fn from(err: google_fcm1::Error) -> Self { match err { google_fcm1::Error::BadRequest(ref value) => { - if value["error"]["status"].as_str() == Some("NOT_FOUND") + if value["error"]["code"].as_u64() == Some(404) + && value["error"]["status"].as_str() == Some("NOT_FOUND") && value["error"]["details"] .as_array() .map_or(false, |details| { From b7035ede4da8fe5143b7f1f61b62999896a5200c Mon Sep 17 00:00:00 2001 From: Vaibhav Date: Wed, 14 Feb 2024 14:13:32 +0530 Subject: [PATCH 3/3] fix(notifications): use safe access methods for JSON parsing --- core/notifications/src/executor/fcm/error.rs | 31 +++++++++++++------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/core/notifications/src/executor/fcm/error.rs b/core/notifications/src/executor/fcm/error.rs index 069d808885..33841f613b 100644 --- a/core/notifications/src/executor/fcm/error.rs +++ b/core/notifications/src/executor/fcm/error.rs @@ -14,17 +14,28 @@ impl From for FcmError { fn from(err: google_fcm1::Error) -> Self { match err { google_fcm1::Error::BadRequest(ref value) => { - if value["error"]["code"].as_u64() == Some(404) - && value["error"]["status"].as_str() == Some("NOT_FOUND") - && value["error"]["details"] - .as_array() - .map_or(false, |details| { - details - .iter() - .any(|detail| detail["errorCode"].as_str() == Some("UNREGISTERED")) + let code = value + .get("error") + .and_then(|e| e.get("code")) + .and_then(|c| c.as_u64()); + let status = value + .get("error") + .and_then(|e| e.get("status")) + .and_then(|s| s.as_str()); + let is_unregistered = value + .get("error") + .and_then(|e| e.get("details")) + .and_then(|d| d.as_array()) + .map_or(false, |details| { + details.iter().any(|detail| { + detail.get("errorCode").and_then(|e| e.as_str()) == Some("UNREGISTERED") }) - { - return FcmError::UnrecognizedDeviceToken(err); + }); + + if let (Some(code), Some(status)) = (code, status) { + if code == 404 && status == "NOT_FOUND" && is_unregistered { + return FcmError::UnrecognizedDeviceToken(err); + } } FcmError::GoogleFcm1Error(err) }