From 33ea5ac0fbde993fd306b542798fb97ed976ab7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 17 Sep 2024 15:58:46 +0200 Subject: [PATCH 1/3] Add retries to apple push notifications --- server/apple_notification_server.go | 62 +++++++++++++++++++++++++---- server/server.go | 1 + 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/server/apple_notification_server.go b/server/apple_notification_server.go index 3a59627..be11473 100644 --- a/server/apple_notification_server.go +++ b/server/apple_notification_server.go @@ -227,15 +227,8 @@ func (me *AppleNotificationServer) SendNotification(msg *PushNotification) PushR if me.AppleClient != nil { me.logger.Infof("Sending apple push notification for device=%v type=%v ackId=%v", me.ApplePushSettings.Type, msg.Type, msg.AckID) - start := time.Now() - - ctx, cancel := context.WithTimeout(context.Background(), me.sendTimeout) - defer cancel() - res, err := me.AppleClient.PushWithContext(ctx, notification) - if me.metrics != nil { - me.metrics.observerNotificationResponse(PushNotifyApple, time.Since(start).Seconds()) - } + res, err := me.SendNotificationWithRetry(notification) if err != nil { me.logger.Errorf("Failed to send apple push sid=%v did=%v err=%v type=%v", msg.ServerID, msg.DeviceID, err, me.ApplePushSettings.Type) if me.metrics != nil { @@ -269,3 +262,56 @@ func (me *AppleNotificationServer) SendNotification(msg *PushNotification) PushR } return NewOkPushResponse() } + +func (me *AppleNotificationServer) SendNotificationWithRetry(notification *apns.Notification) (*apns.Response, error) { + var res *apns.Response + var err error + waitTime := time.Second + + // Keep a general context to make sure the whole retry + // doesn't take longer than the timeout. + generalContext, cancelGeneralContext := context.WithTimeout(context.Background(), me.sendTimeout) + defer cancelGeneralContext() + + // Set the retry context timeout to a value that allow us to retry the notification + // the MAX RETRIES with exponential backoff. With default values, this timeout + // will be 5 seconds. + retryContextTimeout := me.sendTimeout / (MAX_RETRIES * 2) + + for retries := 0; retries < MAX_RETRIES; retries++ { + start := time.Now() + + retryContext, cancelRetryContext := context.WithTimeout(generalContext, retryContextTimeout) + defer cancelRetryContext() + res, err = me.AppleClient.PushWithContext(retryContext, notification) + if me.metrics != nil { + me.metrics.observerNotificationResponse(PushNotifyApple, time.Since(start).Seconds()) + } + + if err == nil { + break + } + + me.logger.Errorf("Failed to send apple push did=%v retry=%v error=%v", notification.DeviceToken, retries, err) + + if retries == MAX_RETRIES-1 { + me.logger.Errorf("Max retries reached did=%v", notification.DeviceToken) + break + } + + select { + case <-generalContext.Done(): + case <-time.After(waitTime): + } + + if generalContext.Err() != nil { + me.logger.Infof("Not retrying because context error did=%v retry=%v error=%v", notification.DeviceToken, retries, generalContext.Err()) + err = generalContext.Err() + break + } + + waitTime *= 2 + } + + return res, err +} diff --git a/server/server.go b/server/server.go index c7e99d4..f2768ea 100644 --- a/server/server.go +++ b/server/server.go @@ -27,6 +27,7 @@ const ( HEADER_REAL_IP = "X-Real-IP" WAIT_FOR_SERVER_SHUTDOWN = time.Second * 5 CONNECTION_TIMEOUT_SECONDS = 60 + MAX_RETRIES = 3 ) type NotificationServer interface { From 6ec7013209749fa668ebd8a8b81d7ffeade5f8be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 19 Sep 2024 16:49:12 +0200 Subject: [PATCH 2/3] Add retry timeout config --- config/mattermost-push-proxy.sample.json | 1 + server/apple_notification_server.go | 11 ++++------- server/config_push_proxy.go | 11 +++++++++++ server/server.go | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/config/mattermost-push-proxy.sample.json b/config/mattermost-push-proxy.sample.json index a6014e0..bfbb466 100644 --- a/config/mattermost-push-proxy.sample.json +++ b/config/mattermost-push-proxy.sample.json @@ -5,6 +5,7 @@ "ThrottleVaryByHeader":"X-Forwarded-For", "EnableMetrics": false, "SendTimeoutSec": 30, + "RetryTimeoutSec": 8, "ApplePushSettings":[ { "Type":"apple", diff --git a/server/apple_notification_server.go b/server/apple_notification_server.go index be11473..e3d0dcc 100644 --- a/server/apple_notification_server.go +++ b/server/apple_notification_server.go @@ -25,14 +25,16 @@ type AppleNotificationServer struct { logger *Logger ApplePushSettings ApplePushSettings sendTimeout time.Duration + retryTimeout time.Duration } -func NewAppleNotificationServer(settings ApplePushSettings, logger *Logger, metrics *metrics, sendTimeoutSecs int) *AppleNotificationServer { +func NewAppleNotificationServer(settings ApplePushSettings, logger *Logger, metrics *metrics, sendTimeoutSecs int, retryTimeoutSecs int) *AppleNotificationServer { return &AppleNotificationServer{ ApplePushSettings: settings, metrics: metrics, logger: logger, sendTimeout: time.Duration(sendTimeoutSecs) * time.Second, + retryTimeout: time.Duration(retryTimeoutSecs) * time.Second, } } @@ -273,15 +275,10 @@ func (me *AppleNotificationServer) SendNotificationWithRetry(notification *apns. generalContext, cancelGeneralContext := context.WithTimeout(context.Background(), me.sendTimeout) defer cancelGeneralContext() - // Set the retry context timeout to a value that allow us to retry the notification - // the MAX RETRIES with exponential backoff. With default values, this timeout - // will be 5 seconds. - retryContextTimeout := me.sendTimeout / (MAX_RETRIES * 2) - for retries := 0; retries < MAX_RETRIES; retries++ { start := time.Now() - retryContext, cancelRetryContext := context.WithTimeout(generalContext, retryContextTimeout) + retryContext, cancelRetryContext := context.WithTimeout(generalContext, me.retryTimeout) defer cancelRetryContext() res, err = me.AppleClient.PushWithContext(retryContext, notification) if me.metrics != nil { diff --git a/server/config_push_proxy.go b/server/config_push_proxy.go index 11b2d94..3c3cafd 100644 --- a/server/config_push_proxy.go +++ b/server/config_push_proxy.go @@ -17,6 +17,7 @@ type ConfigPushProxy struct { ThrottleVaryByHeader string LogFileLocation string SendTimeoutSec int + RetryTimeoutSec int ApplePushSettings []ApplePushSettings EnableMetrics bool EnableConsoleLog bool @@ -81,6 +82,16 @@ func LoadConfig(fileName string) (*ConfigPushProxy, error) { if !cfg.EnableConsoleLog && !cfg.EnableFileLog { cfg.EnableConsoleLog = true } + + // Set timeout defaults + if cfg.SendTimeoutSec == 0 { + cfg.SendTimeoutSec = 30 + } + + if cfg.RetryTimeoutSec == 0 { + cfg.RetryTimeoutSec = 8 + } + if cfg.EnableFileLog { if cfg.LogFileLocation == "" { // We just do an mkdir -p equivalent. diff --git a/server/server.go b/server/server.go index f2768ea..986be48 100644 --- a/server/server.go +++ b/server/server.go @@ -70,7 +70,7 @@ func (s *Server) Start() { } for _, settings := range s.cfg.ApplePushSettings { - server := NewAppleNotificationServer(settings, s.logger, m, s.cfg.SendTimeoutSec) + server := NewAppleNotificationServer(settings, s.logger, m, s.cfg.SendTimeoutSec, s.cfg.RetryTimeoutSec) err := server.Initialize() if err != nil { s.logger.Errorf("Failed to initialize client: %v", err) From def0093a860a3a13db6eac2ce057ce58c8652843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 24 Sep 2024 12:45:09 +0200 Subject: [PATCH 3/3] Add check for retry timeout being greater than send timeout --- server/config_push_proxy.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/config_push_proxy.go b/server/config_push_proxy.go index 3c3cafd..0fe8836 100644 --- a/server/config_push_proxy.go +++ b/server/config_push_proxy.go @@ -92,6 +92,10 @@ func LoadConfig(fileName string) (*ConfigPushProxy, error) { cfg.RetryTimeoutSec = 8 } + if cfg.RetryTimeoutSec > cfg.SendTimeoutSec { + cfg.RetryTimeoutSec = cfg.SendTimeoutSec + } + if cfg.EnableFileLog { if cfg.LogFileLocation == "" { // We just do an mkdir -p equivalent.