Skip to content

Commit a4adb82

Browse files
nan-licursoragent
andcommitted
fix: [SDK-4792] guard push subscriptions against transient token-fetch errors
A tokenless, transient FCM/HMS token-fetch error (e.g. -9, -11) could overwrite an already-SUBSCRIBED push subscription and flip it to unsubscribed on the backend. The prior guard lived in PushTokenManager using in-memory state that reset on process restart, so a cold start during an FCM/HMS outage could still persist the error. - Add SubscriptionStatus.isRetryableTokenError covering the transient FCM/HMS codes (-8, -9, -11, -12, -25, -27, -29). - Add a durable guard in SubscriptionManager.addOrUpdatePushSubscriptionToken that ignores a tokenless retryable error when the persisted subscription is already SUBSCRIBED with a valid token. Real opt-outs / permission changes are not retryable and still downgrade as before. - Add tests for the guard and the new flag; correct an existing test that paired a non-null token with an error status (an impossible production combination). Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent ff1e8be commit a4adb82

3 files changed

Lines changed: 217 additions & 3 deletions

File tree

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModel.kt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,33 @@ enum class SubscriptionStatus(val value: Int) {
7474
ERROR(9999),
7575
;
7676

77+
/**
78+
* `true` when this status represents a *transient, retryable* failure to fetch a push token
79+
* from FCM/HMS (e.g. a temporary IOException, service-unavailable, or init error)
80+
*
81+
* A retryable token error means "we momentarily couldn't reach FCM/HMS", not "this device can
82+
* no longer receive push". It must therefore never downgrade a subscription that is already
83+
* [SUBSCRIBED] with a valid token — the next successful registration recovers the token.
84+
*/
85+
val isRetryableTokenError: Boolean
86+
get() = this in RETRYABLE_TOKEN_ERRORS
87+
7788
companion object {
89+
/**
90+
* Transient, retryable token-fetch failures. These are the statuses that should not
91+
* overwrite a healthy push subscription.
92+
*/
93+
private val RETRYABLE_TOKEN_ERRORS =
94+
setOf(
95+
FIREBASE_FCM_INIT_ERROR, // -8
96+
FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE, // -9
97+
FIREBASE_FCM_ERROR_IOEXCEPTION_OTHER, // -11
98+
FIREBASE_FCM_ERROR_MISC_EXCEPTION, // -12
99+
HMS_TOKEN_TIMEOUT, // -25
100+
HMS_API_EXCEPTION_OTHER, // -27
101+
FIREBASE_FCM_ERROR_IOEXCEPTION_AUTHENTICATION_FAILED, // -29
102+
)
103+
78104
fun fromInt(value: Int): SubscriptionStatus? {
79105
return SubscriptionStatus.values().firstOrNull { it.value == value }
80106
}

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/impl/SubscriptionManager.kt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,19 @@ internal class SubscriptionManager(
8989
pushSubModel.address = pushToken
9090
}
9191

92+
// A temporary token-fetch failure (no token) shouldn't unsubscribe a device that is
93+
// already subscribed with a valid token. Keep the current status.
94+
val isTransientTokenlessError = pushToken == null && pushTokenStatus.isRetryableTokenError
95+
val isAlreadyHealthy =
96+
pushSubModel.status == SubscriptionStatus.SUBSCRIBED && pushSubModel.address.isNotEmpty()
97+
if (isTransientTokenlessError && isAlreadyHealthy) {
98+
Logging.warn(
99+
"SubscriptionManager: ignoring transient push token status $pushTokenStatus " +
100+
"(${pushTokenStatus.value}).",
101+
)
102+
return
103+
}
104+
92105
pushSubModel.status = pushTokenStatus
93106
}
94107
}

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt

Lines changed: 178 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import com.onesignal.common.PIIHasher
55
import com.onesignal.common.modeling.ModelChangeTags
66
import com.onesignal.common.modeling.ModelChangedArgs
77
import com.onesignal.core.internal.application.IApplicationService
8+
import com.onesignal.debug.LogLevel
9+
import com.onesignal.debug.internal.logging.Logging
810
import com.onesignal.session.internal.session.ISessionService
911
import com.onesignal.user.internal.Subscription
1012
import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
@@ -24,6 +26,10 @@ import io.mockk.verify
2426

2527
class SubscriptionManagerTests : FunSpec({
2628

29+
beforeAny {
30+
Logging.logLevel = LogLevel.NONE
31+
}
32+
2733
test("initializes subscriptions from model store") {
2834
// Given
2935
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
@@ -182,12 +188,13 @@ class SubscriptionManagerTests : FunSpec({
182188

183189
val subscriptionManager = SubscriptionManager(mockApplicationService, mockSessionService, mockSubscriptionModelStore)
184190

185-
// When
186-
subscriptionManager.addOrUpdatePushSubscriptionToken("pushToken2", SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_OTHER)
191+
// When a new token arrives (a successful registration always reports SUBSCRIBED with a
192+
// token; an error always reports a null token, never a token paired with an error status)
193+
subscriptionManager.addOrUpdatePushSubscriptionToken("pushToken2", SubscriptionStatus.SUBSCRIBED)
187194

188195
// Then
189196
pushSubscription.address shouldBe "pushToken2"
190-
pushSubscription.status shouldBe SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_OTHER
197+
pushSubscription.status shouldBe SubscriptionStatus.SUBSCRIBED
191198
}
192199

193200
test("remove email subscription removes from model store") {
@@ -617,4 +624,172 @@ class SubscriptionManagerTests : FunSpec({
617624
result shouldNotBe null
618625
result!!.id shouldBe "subscription1"
619626
}
627+
628+
// A transient, tokenless FCM/HMS token-fetch failure (e.g. -9 SERVICE_NOT_AVAILABLE) must not
629+
// downgrade a push subscription that is already SUBSCRIBED with a valid cached token. A failed
630+
// registration reports a null token alongside the error status, so the cached address is left
631+
// intact and persisting the error would spuriously unsubscribe the device on the backend.
632+
test("transient tokenless token-fetch error does not downgrade a healthy SUBSCRIBED push subscription") {
633+
val retryableErrors =
634+
listOf(
635+
SubscriptionStatus.FIREBASE_FCM_INIT_ERROR,
636+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE,
637+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_OTHER,
638+
SubscriptionStatus.FIREBASE_FCM_ERROR_MISC_EXCEPTION,
639+
SubscriptionStatus.HMS_TOKEN_TIMEOUT,
640+
SubscriptionStatus.HMS_API_EXCEPTION_OTHER,
641+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_AUTHENTICATION_FAILED,
642+
)
643+
644+
for (status in retryableErrors) {
645+
// Given a healthy push subscription
646+
val pushSubscription = SubscriptionModel()
647+
pushSubscription.id = "subscription1"
648+
pushSubscription.type = SubscriptionType.PUSH
649+
pushSubscription.status = SubscriptionStatus.SUBSCRIBED
650+
pushSubscription.optedIn = true
651+
pushSubscription.address = "validToken"
652+
653+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
654+
val mockApplicationService = mockk<IApplicationService>()
655+
val mockSessionService = mockk<ISessionService>(relaxed = true)
656+
every { mockSubscriptionModelStore.subscribe(any()) } just runs
657+
every { mockSubscriptionModelStore.list() } returns listOf(pushSubscription)
658+
659+
val subscriptionManager =
660+
SubscriptionManager(mockApplicationService, mockSessionService, mockSubscriptionModelStore)
661+
662+
// When a failed registration reports a null token alongside the transient error
663+
subscriptionManager.addOrUpdatePushSubscriptionToken(null, status)
664+
665+
// Then the healthy SUBSCRIBED state and cached token are preserved
666+
pushSubscription.status shouldBe SubscriptionStatus.SUBSCRIBED
667+
pushSubscription.address shouldBe "validToken"
668+
}
669+
}
670+
671+
// The guard must be narrow: real opt-outs / permission changes are also tokenless, but they are
672+
// not retryable token errors and must continue to downgrade the subscription as before.
673+
test("non-retryable tokenless statuses still downgrade a SUBSCRIBED push subscription") {
674+
val nonRetryableStatuses =
675+
listOf(
676+
SubscriptionStatus.NO_PERMISSION,
677+
SubscriptionStatus.UNSUBSCRIBE,
678+
SubscriptionStatus.DISABLED_FROM_REST_API_DEFAULT_REASON,
679+
)
680+
681+
for (status in nonRetryableStatuses) {
682+
// Given a healthy push subscription
683+
val pushSubscription = SubscriptionModel()
684+
pushSubscription.id = "subscription1"
685+
pushSubscription.type = SubscriptionType.PUSH
686+
pushSubscription.status = SubscriptionStatus.SUBSCRIBED
687+
pushSubscription.optedIn = true
688+
pushSubscription.address = "validToken"
689+
690+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
691+
val mockApplicationService = mockk<IApplicationService>()
692+
val mockSessionService = mockk<ISessionService>(relaxed = true)
693+
every { mockSubscriptionModelStore.subscribe(any()) } just runs
694+
every { mockSubscriptionModelStore.list() } returns listOf(pushSubscription)
695+
696+
val subscriptionManager =
697+
SubscriptionManager(mockApplicationService, mockSessionService, mockSubscriptionModelStore)
698+
699+
// When
700+
subscriptionManager.addOrUpdatePushSubscriptionToken(null, status)
701+
702+
// Then the downgrade is applied
703+
pushSubscription.status shouldBe status
704+
}
705+
}
706+
707+
test("transient tokenless token-fetch error is persisted when there is no cached token to protect") {
708+
// Given a SUBSCRIBED push subscription with no cached token (nothing to protect)
709+
val pushSubscription = SubscriptionModel()
710+
pushSubscription.id = "subscription1"
711+
pushSubscription.type = SubscriptionType.PUSH
712+
pushSubscription.status = SubscriptionStatus.SUBSCRIBED
713+
pushSubscription.optedIn = true
714+
pushSubscription.address = ""
715+
716+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
717+
val mockApplicationService = mockk<IApplicationService>()
718+
val mockSessionService = mockk<ISessionService>(relaxed = true)
719+
every { mockSubscriptionModelStore.subscribe(any()) } just runs
720+
every { mockSubscriptionModelStore.list() } returns listOf(pushSubscription)
721+
722+
val subscriptionManager =
723+
SubscriptionManager(mockApplicationService, mockSessionService, mockSubscriptionModelStore)
724+
725+
// When
726+
subscriptionManager.addOrUpdatePushSubscriptionToken(
727+
null,
728+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE,
729+
)
730+
731+
// Then the error is persisted (the guard only protects an already-healthy, tokened subscription)
732+
pushSubscription.status shouldBe SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE
733+
}
734+
735+
test("transient tokenless token-fetch error is persisted when the subscription is not already SUBSCRIBED") {
736+
// Given a push subscription that is already in a non-subscribed state
737+
val pushSubscription = SubscriptionModel()
738+
pushSubscription.id = "subscription1"
739+
pushSubscription.type = SubscriptionType.PUSH
740+
pushSubscription.status = SubscriptionStatus.NO_PERMISSION
741+
pushSubscription.optedIn = true
742+
pushSubscription.address = "validToken"
743+
744+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
745+
val mockApplicationService = mockk<IApplicationService>()
746+
val mockSessionService = mockk<ISessionService>(relaxed = true)
747+
every { mockSubscriptionModelStore.subscribe(any()) } just runs
748+
every { mockSubscriptionModelStore.list() } returns listOf(pushSubscription)
749+
750+
val subscriptionManager =
751+
SubscriptionManager(mockApplicationService, mockSessionService, mockSubscriptionModelStore)
752+
753+
// When
754+
subscriptionManager.addOrUpdatePushSubscriptionToken(
755+
null,
756+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE,
757+
)
758+
759+
// Then the status is updated (there is no healthy SUBSCRIBED state to protect)
760+
pushSubscription.status shouldBe SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE
761+
}
762+
763+
test("SubscriptionStatus.isRetryableTokenError flags exactly the transient token-fetch errors") {
764+
// Given the full enum
765+
// When filtering by the new flag
766+
val retryable = SubscriptionStatus.values().filter { it.isRetryableTokenError }.toSet()
767+
768+
// Then it is exactly the transient FCM/HMS token-fetch errors and nothing else
769+
retryable shouldBe
770+
setOf(
771+
SubscriptionStatus.FIREBASE_FCM_INIT_ERROR,
772+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_SERVICE_NOT_AVAILABLE,
773+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_OTHER,
774+
SubscriptionStatus.FIREBASE_FCM_ERROR_MISC_EXCEPTION,
775+
SubscriptionStatus.HMS_TOKEN_TIMEOUT,
776+
SubscriptionStatus.HMS_API_EXCEPTION_OTHER,
777+
SubscriptionStatus.FIREBASE_FCM_ERROR_IOEXCEPTION_AUTHENTICATION_FAILED,
778+
)
779+
}
780+
781+
test("SubscriptionStatus.isRetryableTokenError is false for healthy and user-driven statuses") {
782+
// SUBSCRIBED, permission/opt-out, and permanent configuration errors must never be treated
783+
// as transient, otherwise the guard would mask real subscription state changes.
784+
listOf(
785+
SubscriptionStatus.SUBSCRIBED,
786+
SubscriptionStatus.NO_PERMISSION,
787+
SubscriptionStatus.UNSUBSCRIBE,
788+
SubscriptionStatus.INVALID_FCM_SENDER_ID,
789+
SubscriptionStatus.OUTDATED_GOOGLE_PLAY_SERVICES_APP,
790+
SubscriptionStatus.HMS_ARGUMENTS_INVALID,
791+
SubscriptionStatus.DISABLED_FROM_REST_API_DEFAULT_REASON,
792+
SubscriptionStatus.ERROR,
793+
).forEach { it.isRetryableTokenError shouldBe false }
794+
}
620795
})

0 commit comments

Comments
 (0)