mozilla
[push] Create a UAID for the device on first initialization
Related A-C issue: https://github.com/mozilla-mobile/android-components/issues/5648
When migrating from Fennec to Fenix, we're receiving new push registration tokens from Firebase before we have a first subscription created. Because of this we call PushManager.update before a PushManager.subscribe has taken place.
Currently, a valid code path would be: PushManager.init ~~> PushManager.subscribe~~ > PushManager.update.
What we're seeing is: PushManager.init -> PushManager.update.
┆Issue is synchronized with this Jira Task ┆Epic: FxA Ecosystem (backlog)
Nice work on debugging this! It's not 100% clear to me what change we should be making in a-s to help with this, should we e.g. be throwing an error if you call PushManager.update before calling PushManager.subscribe, or do we need to rejigger things so that calling update before subscribe does something useful?
Semi-answering my own question...so IIUC, subscribe currently lazily-initializes a uaid for the push manager when a subscription is created for the first time, but we should either do this earlier (e.g. in the constructor) or add it to other method calls that assume one already exists. Does that sound right?
It's not so lazily, since subscribe is the only way to get a new UAID from the server. The protocol just distinguishes new requests from existing requests internally via an "auth". What's supposed to happen is:
- client gets local
request_idfrom the OS' native push system - client instantiates a new
PushManagerobject passing the initialization values. - when a new subscription request happens, client makes the new
subscription()request which initializes the server. - much later, when the OS notifies the app that the
request_idhas changed, client callsPushManager.update()with the newrequest_id
In reality these steps don't happen. What generally happens is that during app initialization, the app either fetches or receives the request_id, instantiates the PushManager(1), then calls update with the request_id which fails because the PushManager internal state has not been initialized by the first subscription() request.
The larger problem is that this is not the first time that devs have called update() on an uninitialized connection and gotten an error back. It's clearly a desired pattern and it makes sense to pave that particular goat path.
To smooth the API out, instead of raising the error, update should simply create a semi-bogus subscription() request, which will initialize the server, log the new client request_id, and return a uaid. The subscription endpoint could be discarded, since there will not be a recipient process.
(1)This is partly due to a FCM protocol change, which happened after much of the original server protocol and rust module was built, so I can't really blame the client folk here.
While doing some WebPush integration testing it became clearer to me, this can happen way more often than I imagined.
A user may not be:
- An FxA user - so no subscription there.
- A WebPush user - because not everyone wants notifications.
A token from FCM can come at any point in time before either of those events. So I think we've been getting lucky so far then by not having run into this case before.
(1)This is partly due to a FCM protocol change, which happened after much of the original server protocol and rust module was built, so I can't really blame the client folk here.
The one time I finally come out clean! 🎉
We've been lucky with this so far since we immediately create a subscription for FxA regardless of if you have an FxA account. We can keep that bit of code in for now as a dummy subscription to avoid this case.
Thinking about this more. It seems like the only reason we fail hard over here is because we want to store the token (native_id in this context) along with the UAID. We could separate these two and store the native_id separately.
When we subscribe then, failing hard is a more acceptable place (and less likely to happen since it our happy code path ensures having the UAID first). This would simplify the change without needing to make any fake subscriptions in order to retrieve the UAID.
Thinking about this more. It seems like the only reason we fail hard over here is because we want to store the token (
native_idin this context) along with the UAID. We could separate these two and store thenative_idseparately.
Yes, I think this is the correct approach. ISTM that a very common case will be no subscriptions. While we are in a state of no subscriptions, it's not necessary for us to have ever spoken to the server, so not having a UAID in this case seems the best possible outcome. Once we get our first "real" subscription we should be able to bootstrap the world at that time (ie, get the UAID by way of that first subscription, then callung update with the FCM token.
(Relatedly, ISTM that rust code is quite confused - the schema appears to be looking at allowing the possibility that different subscriptions might actually have different UAIDs, and even that the same channel ID can be used with different UAIDs - both of which I think is false. However, the "good" news is that verify_connections is quite brutal in that it discards all subscriptions on any change, which I think means in practice we would blow the entire DB up and start again in the case of a UAID change - but it's confusing, and only partially related to this ticket, so I'll stop now :)
Moved to bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1865156
Change performed by the Move to Bugzilla add-on.