Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Renewing push registration should trigger the onSubscriptionChanged handler #7143

Closed
rfk opened this issue May 28, 2020 · 12 comments
Closed
Assignees
Labels
blocked Blocked pull requests and issues 🐞 bug Something isn't working <push> Components: concept-push, lib-push-adm, lib-push-firebase, feature-push, feature-sendtab

Comments

@rfk
Copy link
Contributor

rfk commented May 28, 2020

I'm spinning this off from a discussion in an application-services issue here.

I have an install of Firefox Beta that is not correctly recovering from an expired push subscription with the FxA server.

There's some logic in the FxaPushSupportFeature that detects when the device's push subscription has been marked as expired by the FxA server (here), and logcat tells me that my Fenix is correctly triggering this code. However, the updated subscription is not being communicated back to the FxA server.

IIUC, the uploading of new subscription details is supposed to be handled by this onSubscriptionChanged hander in FxaPushSupportFeature, but my logcat output doesn't show any evidence of this being triggered.

Am I right to expect that onSubscriptionChanged should be triggered here? Or, is there another mechanism by which the new subscription details should be communicated back to the FxA server?

/cc @jonalmeida

┆Issue is synchronized with this Jira Task

@rfk
Copy link
Contributor Author

rfk commented May 28, 2020

(In terms of user impact, this means that send-tab is in a degraded state for this device - I'm able to send tabs to it from my other devices, but it won't receive the tabs until I do an explicit "sync now").

@jonalmeida jonalmeida added <push> Components: concept-push, lib-push-adm, lib-push-firebase, feature-push, feature-sendtab 🔬 Research Problems without a solution that need research. labels May 28, 2020
@jonalmeida jonalmeida self-assigned this May 28, 2020
@jonalmeida jonalmeida added the 🐞 bug Something isn't working label Jun 1, 2020
@jonalmeida
Copy link
Contributor

After doing some investigation today I've confirmed that we are requesting a subscription verification check when the app opens but we are not getting any new subscription changes back; JR is looking into this in mozilla-services/autopush#1397.

I did file two other issues as a result of this investigation:

  1. We fail to verify subscriptions on the first launch of the app: Push subscription verification is not always executed on first install #7236
  2. We can do a verification check immediately after receiving the push token instead of waiting for the next initialization: Check subscription verification immediately after getting a new push token #7237

@rfk
Copy link
Contributor Author

rfk commented Jun 4, 2020

After doing some investigation today I've confirmed that we are requesting a subscription verification check when the app opens

Thanks for much for digging into this @jonalmeida. One thing that's not quite clear to me - after renewing the push registration, do we wait until the next app restart in order to verify the subscription list with autopush, or will we do it in the background while the user is still using the app?

@jonalmeida
Copy link
Contributor

jonalmeida commented Jun 4, 2020

@rfk We wait until the next app restart in order to verify the subscription list. To be clear, we attempt to check on every app cold start, so that's where this check happens. Point 2, in my comment above is to change this. Although, I'd like to fix this bug first before we change the behaviour.

@jonalmeida jonalmeida added the blocked Blocked pull requests and issues label Jun 5, 2020
@jonalmeida
Copy link
Contributor

For clarity on the ticket, we're blocking this from closing on investigation from mozilla-services/autopush#1397 which may require changes in app-services and/or android-components.

@tublitzed
Copy link

hey, @jonalmeida ^ just caught wind of this. We'll get mozilla-services/autopush#1397 into the queue next week (team meeting Monday) and should be able to update you with an ETA shortly after that.

@data-sync-user data-sync-user changed the title Renewing push registration should trigger the onSubscriptionChanged handler FNX3-22736 ⁃ Renewing push registration should trigger the onSubscriptionChanged handler Aug 4, 2020
@data-sync-user data-sync-user changed the title FNX3-22736 ⁃ Renewing push registration should trigger the onSubscriptionChanged handler FNX2-18208 ⁃ Renewing push registration should trigger the onSubscriptionChanged handler Aug 5, 2020
@st3fan st3fan changed the title FNX2-18208 ⁃ Renewing push registration should trigger the onSubscriptionChanged handler Renewing push registration should trigger the onSubscriptionChanged handler Aug 5, 2020
@jonalmeida
Copy link
Contributor

We've ignored this issue for far too long and it's begun to bite us now: mozilla-mobile/fenix#15899

@rfk
Copy link
Contributor Author

rfk commented Oct 19, 2020

Is progress here still blocked on mozilla/application-services#3314?

@jonalmeida
Copy link
Contributor

Yes.

For the ticket, we're discussing this more in the above linked ticket.

@jonalmeida jonalmeida removed the 🔬 Research Problems without a solution that need research. label Oct 26, 2020
@jonalmeida
Copy link
Contributor

This issue now blocks #4819 (comment) which in turn blocks #8459.

@jonalmeida
Copy link
Contributor

Using the latest version of Firebase Crashlytics is also blocked on upgrading FCM (which is blocked on this issue).

To unblock ourselves, we can come up with a patch that specifically fixes the WebPush subscription for Send Tab when FxA has an invalid subscription and force-resubscribe immediately by passing the observer for onSubscriptionChanged.

This is good for FxA/Send Tab but leaves web content without a fix, so we still need to fix this issue.

jonalmeida added a commit to jonalmeida/android-components that referenced this issue Nov 3, 2020
…criptionExpired

This is a workaround fix to re-enable Send Tab when we are notified
about the subscriptionExpired flag. Until mozilla-mobile#7143 is fixed, we need to
avoid calling `registrationRenewal` to avoid going into an unfixable
state.

In the context of the FxaPushSupportFeature, we know that our end goal
is to call `push.subscribe(fxaPushScope)`, so we can by pass the
`verifyConnection` call **assuming** that our native push client is
always has the latest push token and knows how to invalidate the
endpoint with the Autopush server.
jonalmeida added a commit to jonalmeida/android-components that referenced this issue Nov 16, 2020
…criptionExpired

This is a workaround fix to re-enable Send Tab when we are notified
about the subscriptionExpired flag. Until mozilla-mobile#7143 is fixed, we need to
avoid calling `registrationRenewal` to avoid going into an unfixable
state.

In the context of the FxaPushSupportFeature, we know that our end goal
is to call `push.subscribe(fxaPushScope)`, so we can by pass the
`verifyConnection` call **assuming** that our native push client is
always has the latest push token and knows how to invalidate the
endpoint with the Autopush server.
jonalmeida added a commit to jonalmeida/android-components that referenced this issue Nov 17, 2020
…criptionExpired

This is a workaround fix to re-enable Send Tab when we are notified
about the subscriptionExpired flag. Until mozilla-mobile#7143 is fixed, we need to
avoid calling `registrationRenewal` to avoid going into an unfixable
state.

In the context of the FxaPushSupportFeature, we know that our end goal
is to call `push.subscribe(fxaPushScope)`, so we can by pass the
`verifyConnection` call **assuming** that our native push client is
always has the latest push token and knows how to invalidate the
endpoint with the Autopush server.
jonalmeida added a commit to jonalmeida/android-components that referenced this issue Nov 17, 2020
…criptionExpired

This is a workaround fix to re-enable Send Tab when we are notified
about the subscriptionExpired flag. Until mozilla-mobile#7143 is fixed, we need to
avoid calling `registrationRenewal` to avoid going into an unfixable
state.

In the context of the FxaPushSupportFeature, we know that our end goal
is to call `push.subscribe(fxaPushScope)`, so we can by pass the
`verifyConnection` call **assuming** that our native push client is
always has the latest push token and knows how to invalidate the
endpoint with the Autopush server.
mergify bot pushed a commit that referenced this issue Nov 17, 2020
This is a workaround fix to re-enable Send Tab when we are notified
about the subscriptionExpired flag. Until #7143 is fixed, we need to
avoid calling `registrationRenewal` to avoid going into an unfixable
state.

In the context of the FxaPushSupportFeature, we know that our end goal
is to call `push.subscribe(fxaPushScope)`, so we can by pass the
`verifyConnection` call **assuming** that our native push client is
always has the latest push token and knows how to invalidate the
endpoint with the Autopush server.
@mhammond
Copy link
Contributor

mhammond commented Oct 15, 2021

I'm fairly confident this is fixed by #11070

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocked Blocked pull requests and issues 🐞 bug Something isn't working <push> Components: concept-push, lib-push-adm, lib-push-firebase, feature-push, feature-sendtab
Projects
None yet
Development

No branches or pull requests

4 participants