Skip to content

Commit 620c92f

Browse files
authored
Add android retry (#131)
* Add android retry * Fix lint * Address feedback * Address feedback
1 parent 7d33b02 commit 620c92f

File tree

3 files changed

+80
-14
lines changed

3 files changed

+80
-14
lines changed

server/android_notification_server.go

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type AndroidNotificationServer struct {
4343
AndroidPushSettings AndroidPushSettings
4444
client *messaging.Client
4545
sendTimeout time.Duration
46+
retryTimeout time.Duration
4647
}
4748

4849
// serviceAccount contains a subset of the fields in service-account.json.
@@ -56,12 +57,13 @@ type serviceAccount struct {
5657
TokenURI string `json:"token_uri"`
5758
}
5859

59-
func NewAndroidNotificationServer(settings AndroidPushSettings, logger *mlog.Logger, metrics *metrics, sendTimeoutSecs int) *AndroidNotificationServer {
60+
func NewAndroidNotificationServer(settings AndroidPushSettings, logger *mlog.Logger, metrics *metrics, sendTimeoutSecs int, retryTimeoutSecs int) *AndroidNotificationServer {
6061
return &AndroidNotificationServer{
6162
AndroidPushSettings: settings,
6263
metrics: metrics,
6364
logger: logger,
6465
sendTimeout: time.Duration(sendTimeoutSecs) * time.Second,
66+
retryTimeout: time.Duration(retryTimeoutSecs) * time.Second,
6567
}
6668
}
6769

@@ -168,21 +170,13 @@ func (me *AndroidNotificationServer) SendNotification(msg *PushNotification) Pus
168170
},
169171
}
170172

171-
ctx, cancel := context.WithTimeout(context.Background(), me.sendTimeout)
172-
defer cancel()
173-
174173
me.logger.Info(
175174
"Sending android push notification",
176175
mlog.String("device", me.AndroidPushSettings.Type),
177176
mlog.String("type", msg.Type),
178177
mlog.String("ack_id", msg.AckID),
179178
)
180-
181-
start := time.Now()
182-
_, err := me.client.Send(ctx, fcmMsg)
183-
if me.metrics != nil {
184-
me.metrics.observerNotificationResponse(PushNotifyAndroid, time.Since(start).Seconds())
185-
}
179+
err := me.SendNotificationWithRetry(fcmMsg)
186180

187181
if err != nil {
188182
errorCode, hasStatusCode := getErrorCode(err)
@@ -240,6 +234,78 @@ func (me *AndroidNotificationServer) SendNotification(msg *PushNotification) Pus
240234
return NewOkPushResponse()
241235
}
242236

237+
func (me *AndroidNotificationServer) SendNotificationWithRetry(fcmMsg *messaging.Message) error {
238+
var err error
239+
waitTime := time.Second
240+
241+
logger := me.logger.With(mlog.String("did", fcmMsg.Token))
242+
243+
// Keep a general context to make sure the whole retry
244+
// doesn't take longer than the timeout.
245+
generalContext, cancelGeneralContext := context.WithTimeout(context.Background(), me.sendTimeout)
246+
defer cancelGeneralContext()
247+
248+
for retries := 0; retries < MAX_RETRIES; retries++ {
249+
start := time.Now()
250+
251+
retryContext, cancelRetryContext := context.WithTimeout(generalContext, me.retryTimeout)
252+
defer cancelRetryContext()
253+
_, err = me.client.Send(retryContext, fcmMsg)
254+
if me.metrics != nil {
255+
me.metrics.observerNotificationResponse(PushNotifyAndroid, time.Since(start).Seconds())
256+
}
257+
258+
if err == nil || !isRetryable(err) {
259+
break
260+
}
261+
262+
logger.Error(
263+
"Failed to send android push",
264+
mlog.Int("retry", retries),
265+
mlog.Err(err),
266+
)
267+
268+
if retries == MAX_RETRIES-1 {
269+
logger.Error("Max retries reached")
270+
break
271+
}
272+
273+
select {
274+
case <-generalContext.Done():
275+
if generalContext.Err() != nil {
276+
logger.Info(
277+
"Not retrying because context error",
278+
mlog.Int("retry", retries),
279+
mlog.Err(generalContext.Err()),
280+
)
281+
}
282+
return generalContext.Err()
283+
case <-time.After(waitTime):
284+
}
285+
286+
waitTime *= 2
287+
}
288+
289+
return err
290+
}
291+
292+
func isRetryable(err error) bool {
293+
// We retry if the context deadline is exceeded.
294+
// This may cause double notifications, but we expect
295+
// this not to happen often.
296+
if errors.Is(err, context.DeadlineExceeded) {
297+
return true
298+
}
299+
300+
// We retry the errors based on https://firebase.google.com/docs/cloud-messaging/http-server-ref
301+
return messaging.IsInternal(err) ||
302+
messaging.IsQuotaExceeded(err)
303+
304+
// messaging.IsUnavailable is retried by the default retry config in
305+
// firebase.google.com/go/[email protected]/internal/http_client.go
306+
// messaging.IsUnavailable(err)
307+
}
308+
243309
func getErrorCode(err error) (string, bool) {
244310
if err == nil {
245311
return "", false

server/android_notification_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TestAndroidInitialize(t *testing.T) {
2626
// Verify error for no service file
2727
pushSettings := AndroidPushSettings{}
2828
cfg.AndroidPushSettings[0] = pushSettings
29-
require.Error(t, NewAndroidNotificationServer(cfg.AndroidPushSettings[0], logger, nil, cfg.SendTimeoutSec).Initialize())
29+
require.Error(t, NewAndroidNotificationServer(cfg.AndroidPushSettings[0], logger, nil, cfg.SendTimeoutSec, cfg.RetryTimeoutSec).Initialize())
3030

3131
f, err := os.CreateTemp("", "example")
3232
require.NoError(t, err)
@@ -37,7 +37,7 @@ func TestAndroidInitialize(t *testing.T) {
3737
// Verify error for bad JSON
3838
_, err = f.Write([]byte("badJSON"))
3939
require.NoError(t, err)
40-
require.Error(t, NewAndroidNotificationServer(cfg.AndroidPushSettings[0], logger, nil, cfg.SendTimeoutSec).Initialize())
40+
require.Error(t, NewAndroidNotificationServer(cfg.AndroidPushSettings[0], logger, nil, cfg.SendTimeoutSec, cfg.RetryTimeoutSec).Initialize())
4141

4242
require.NoError(t, f.Truncate(0))
4343
_, err = f.Seek(0, 0)
@@ -49,7 +49,7 @@ func TestAndroidInitialize(t *testing.T) {
4949
ProjectID: "sample",
5050
}))
5151
require.NoError(t, f.Sync())
52-
require.NoError(t, NewAndroidNotificationServer(cfg.AndroidPushSettings[0], logger, nil, cfg.SendTimeoutSec).Initialize())
52+
require.NoError(t, NewAndroidNotificationServer(cfg.AndroidPushSettings[0], logger, nil, cfg.SendTimeoutSec, cfg.RetryTimeoutSec).Initialize())
5353

5454
require.NoError(t, f.Close())
5555
}

server/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func (s *Server) Start() {
8181
}
8282

8383
for _, settings := range s.cfg.AndroidPushSettings {
84-
server := NewAndroidNotificationServer(settings, s.logger, m, s.cfg.SendTimeoutSec)
84+
server := NewAndroidNotificationServer(settings, s.logger, m, s.cfg.SendTimeoutSec, s.cfg.RetryTimeoutSec)
8585
err := server.Initialize()
8686
if err != nil {
8787
s.logger.Error("Failed to initialize client", mlog.Err(err))

0 commit comments

Comments
 (0)