From b548665b703318a5e82184a138832b28e55b1197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Fri, 30 Aug 2024 11:08:32 -0700 Subject: [PATCH 1/5] Handle APNS request errors to ensure all device tokens receive a notification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes an issue where having one invalid device token registered can cause other device tokens to not receive push notifications. The issue was fixed by improving error handling around APNS requests Signed-off-by: Daniel D’Aquino Closes: https://github.com/damus-io/damus/issues/2408 --- src/notification_manager/notification_manager.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/notification_manager/notification_manager.rs b/src/notification_manager/notification_manager.rs index 739403d..1f9ed34 100644 --- a/src/notification_manager/notification_manager.rs +++ b/src/notification_manager/notification_manager.rs @@ -379,7 +379,11 @@ impl NotificationManager { let apns_client_mutex_guard = self.apns_client.lock().await; - let _response = apns_client_mutex_guard.send(payload).await?; + + match apns_client_mutex_guard.send(payload).await { + Ok(_response) => {}, + Err(e) => log::error!("Failed to send notification to device token '{}': {}", device_token, e), + } log::info!("Notification sent to device token: {}", device_token); From 9ec713b8079f564f7a892f1affd2a780f805b669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Mon, 2 Sep 2024 15:32:59 -0700 Subject: [PATCH 2/5] Fix empty cache entry state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was an issue where an empty cache entry would get interpreted as a "no cache entry found" situation and cause notepush to always fetch a new mutelist Signed-off-by: Daniel D’Aquino --- src/notification_manager/nostr_event_cache.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/notification_manager/nostr_event_cache.rs b/src/notification_manager/nostr_event_cache.rs index b2b9840..7e6b309 100644 --- a/src/notification_manager/nostr_event_cache.rs +++ b/src/notification_manager/nostr_event_cache.rs @@ -97,8 +97,15 @@ impl Cache { if let Some(entry) = self.mute_lists.get(pubkey) { let entry = entry.clone(); // Clone the Arc to avoid borrowing issues if !entry.is_expired(self.max_age) { - if let Some(event) = entry.event.clone() { - return Ok(event.to_mute_list()); + match &entry.event { + Some(event) => { + log::debug!("Cached mute list for pubkey {} was found", pubkey.to_hex()); + return Ok(event.to_mute_list()); + } + None => { + log::debug!("Empty mute list cache entry for pubkey {}", pubkey.to_hex()); + return Ok(None); + } } } else { log::debug!("Mute list for pubkey {} is expired, removing it from the cache", pubkey.to_hex()); @@ -106,6 +113,7 @@ impl Cache { self.remove_event_from_all_maps(&entry.event); } } + log::debug!("Mute list for pubkey {} not found on cache", pubkey.to_hex()); Err(CacheError::NotFound) } From 11568aa6285142e4c19bb0da30977957a92b7d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Mon, 2 Sep 2024 15:51:32 -0700 Subject: [PATCH 3/5] Make max cache age adjustable in env, and change default to 1 hour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel D’Aquino --- src/main.rs | 1 + src/notepush_env.rs | 9 +++++++++ src/notification_manager/nostr_network_helper.rs | 8 +++++--- src/notification_manager/notification_manager.rs | 3 ++- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index b0d7c3a..a147e89 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,6 +40,7 @@ async fn main() -> Result<(), Box> { env.apns_team_id.clone(), env.apns_environment.clone(), env.apns_topic.clone(), + env.nostr_event_cache_max_age, ) .await .expect("Failed to create notification manager"), diff --git a/src/notepush_env.rs b/src/notepush_env.rs index e4fc43a..3f40f98 100644 --- a/src/notepush_env.rs +++ b/src/notepush_env.rs @@ -6,6 +6,7 @@ const DEFAULT_DB_PATH: &str = "./apns_notifications.db"; const DEFAULT_HOST: &str = "0.0.0.0"; const DEFAULT_PORT: &str = "8000"; const DEFAULT_RELAY_URL: &str = "wss://relay.damus.io"; +const DEFAULT_NOSTR_EVENT_CACHE_MAX_AGE: u64 = 60 * 60; // 1 hour pub struct NotePushEnv { // The path to the Apple private key .p8 file @@ -26,6 +27,8 @@ pub struct NotePushEnv { pub api_base_url: String, // The base URL of where the API server is hosted for NIP-98 auth checks // The URL of the Nostr relay server to connect to for getting mutelists pub relay_url: String, + // The max age of the Nostr event cache, in seconds + pub nostr_event_cache_max_age: std::time::Duration, } impl NotePushEnv { @@ -47,6 +50,11 @@ impl NotePushEnv { _ => a2::client::Endpoint::Sandbox, }; let apns_topic = env::var("APNS_TOPIC")?; + let nostr_event_cache_max_age = env::var("NOSTR_EVENT_CACHE_MAX_AGE") + .unwrap_or(DEFAULT_NOSTR_EVENT_CACHE_MAX_AGE.to_string()) + .parse::() + .map(|s| std::time::Duration::from_secs(s)) + .unwrap_or(std::time::Duration::from_secs(DEFAULT_NOSTR_EVENT_CACHE_MAX_AGE)); Ok(NotePushEnv { apns_private_key_path, @@ -59,6 +67,7 @@ impl NotePushEnv { port, api_base_url, relay_url, + nostr_event_cache_max_age }) } diff --git a/src/notification_manager/nostr_network_helper.rs b/src/notification_manager/nostr_network_helper.rs index 8930a4e..ca7b1bc 100644 --- a/src/notification_manager/nostr_network_helper.rs +++ b/src/notification_manager/nostr_network_helper.rs @@ -6,7 +6,6 @@ use super::nostr_event_cache::Cache; use tokio::time::{timeout, Duration}; const NOTE_FETCH_TIMEOUT: Duration = Duration::from_secs(5); -const CACHE_MAX_AGE: Duration = Duration::from_secs(60); pub struct NostrNetworkHelper { client: Client, @@ -16,12 +15,15 @@ pub struct NostrNetworkHelper { impl NostrNetworkHelper { // MARK: - Initialization - pub async fn new(relay_url: String) -> Result> { + pub async fn new(relay_url: String, cache_max_age: Duration) -> Result> { let client = Client::new(&Keys::generate()); client.add_relay(relay_url.clone()).await?; client.connect().await; - Ok(NostrNetworkHelper { client, cache: Mutex::new(Cache::new(CACHE_MAX_AGE)) }) + Ok(NostrNetworkHelper { + client, + cache: Mutex::new(Cache::new(cache_max_age)), + }) } // MARK: - Answering questions about a user diff --git a/src/notification_manager/notification_manager.rs b/src/notification_manager/notification_manager.rs index 1f9ed34..11ffe56 100644 --- a/src/notification_manager/notification_manager.rs +++ b/src/notification_manager/notification_manager.rs @@ -41,6 +41,7 @@ impl NotificationManager { apns_team_id: String, apns_environment: a2::client::Endpoint, apns_topic: String, + cache_max_age: std::time::Duration, ) -> Result> { let connection = db.get()?; Self::setup_database(&connection)?; @@ -58,7 +59,7 @@ impl NotificationManager { apns_topic, apns_client: Mutex::new(client), db: Mutex::new(db), - nostr_network_helper: NostrNetworkHelper::new(relay_url.clone()).await?, + nostr_network_helper: NostrNetworkHelper::new(relay_url.clone(), cache_max_age).await?, }) } From abb3283eaa270c0f5a66488084e2833ab2e9a145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Wed, 4 Sep 2024 14:25:07 -0700 Subject: [PATCH 4/5] Do not override user info row on registration endpoint if it already exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes an issue where notification settings would be lost after restarting Damus, causing unwanted notifications Signed-off-by: Daniel D’Aquino Closes: https://github.com/damus-io/damus/issues/2417 --- src/api_request_handler.rs | 2 +- src/notification_manager/notification_manager.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/api_request_handler.rs b/src/api_request_handler.rs index b55f646..95c567a 100644 --- a/src/api_request_handler.rs +++ b/src/api_request_handler.rs @@ -253,7 +253,7 @@ impl APIHandler { } // Proceed with the main logic after passing all checks - self.notification_manager.save_user_device_info(pubkey, device_token).await?; + self.notification_manager.save_user_device_info_if_not_present(pubkey, device_token).await?; Ok(APIResponse { status: StatusCode::OK, body: json!({ "message": "User info saved successfully" }), diff --git a/src/notification_manager/notification_manager.rs b/src/notification_manager/notification_manager.rs index 11ffe56..ab3d01b 100644 --- a/src/notification_manager/notification_manager.rs +++ b/src/notification_manager/notification_manager.rs @@ -406,6 +406,17 @@ impl NotificationManager { } // MARK: - User device info and settings + + pub async fn save_user_device_info_if_not_present( + &self, + pubkey: nostr::PublicKey, + device_token: &str, + ) -> Result<(), Box> { + if self.is_pubkey_registered(&pubkey).await? { + return Ok(()); + } + self.save_user_device_info(pubkey, device_token).await + } pub async fn save_user_device_info( &self, From aff6e0f8ba2ec6aa88bc4e35b268e57ce25c7696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Wed, 4 Sep 2024 18:14:41 -0700 Subject: [PATCH 5/5] Improve default fallback reaction formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel D’Aquino Closes: https://github.com/damus-io/damus/issues/2420 --- src/notification_manager/notification_manager.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/notification_manager/notification_manager.rs b/src/notification_manager/notification_manager.rs index ab3d01b..b42b806 100644 --- a/src/notification_manager/notification_manager.rs +++ b/src/notification_manager/notification_manager.rs @@ -397,7 +397,16 @@ impl NotificationManager { nostr_sdk::Kind::TextNote => ("New activity".to_string(), event.content.clone()), nostr_sdk::Kind::EncryptedDirectMessage => ("New direct message".to_string(), "Contents are encrypted".to_string()), nostr_sdk::Kind::Repost => ("Someone reposted".to_string(), event.content.clone()), - nostr_sdk::Kind::Reaction => ("New reaction".to_string(), event.content.clone()), + nostr_sdk::Kind::Reaction => { + let content_text = event.content.clone(); + let formatted_text = match content_text.as_str() { + "" => "❤️", + "+" => "❤️", + "-" => "👎", + _ => content_text.as_str(), + }; + ("New reaction".to_string(), formatted_text.to_string()) + }, nostr_sdk::Kind::ZapPrivateMessage => ("New zap private message".to_string(), "Contents are encrypted".to_string()), nostr_sdk::Kind::ZapReceipt => ("Someone zapped you".to_string(), "".to_string()), _ => ("New activity".to_string(), "".to_string()),