I think the time has finally come to do this one for a few reasons. I've been monitoring at the Sentry reports for mismatches between the app-services state machine and the android state machine. The events have gotten pretty low as a result of several fixes. They're now at about 5 per day, down from about 500 a month ago. From what I can see, the remaining events are caused by 2 cases and both of them represent issues in the android code. The first mismatch is on access token key errors. In this case [the android-components code](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/State.kt#177 transitions directly to the authentication problems state. However, [the application-services code](https://github.com/mozilla/application-services/blob/main/components/fxa-client/src/state_machine/internal_machines/connected.rs#L19) forces you to check the authorization status and only transitions to the auth issues state if that fails. The "authorization status check" means attempting to get a new access token from the refresh token. My feeling is that the android-components code is wrong since there are times where the access token has expired, but you would be able to get a new one using the refresh token. I could be wrong though, maybe the android-components code has identified cases where we know the refresh token is definitely invalid and we can skip the authentication check. However, even if that's true I think the current application-services API is better from our POV. It feels much safer to always do the check an not give the consumer an event that skips it. The worst case scenario is an extra API request to FxA, which seems fine. The second mismatch feels like a pretty bad bug in the android-components code to me. [The code creates a CoroutineContext using Executors.newSingleThreadExecutor](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt#107-111) and only calls `processQueue` inside that context to try to ensure serialized execution of the queue. However, there's an issue with that model: if a task running in that CoroutineContext calls `processQueue` again then the code will run processQueue again while the first one is still running. This is currently happening in this corner case: - firefox-android performs some FxA operation inside processQueue - All the Sentry reports I read were for completing oauth, but I think it could be any operation. - The operation fails with an auth error - Eventually, [encounteredAuthError](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt#440-450) is called which starts another processQueue run (note that the first one is still in the middle of processing) - At this point everything is very weird, the inner process queue will probably cause the state machine to transition to the auth problems state. Then the first process queue call will then continue and probable error out as well, but I suspect that there can be some subtle/weird bugs that happen in this process. To summarize, I think the state machine process has found all the bugs in the app-services code it's going to find and it's now revealing bugs in the android-components code that should be fixed. The simplest way to fix them would be to migrate to using the app-services code. The other reason I want to make the switch now is https://bugzilla.mozilla.org/show_bug.cgi?id=1887071. I believe we can fix that without too much work, but for a while I was worried that we would need to know that the client transitioned to the auth issues state in order to fix it. This makes me think there are potential bugs out there that would be easier to fix if the consumers were using our state machine rather than theirs.
Bug 1867793 Comment 2 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
I think the time has finally come to do this one for a few reasons. I've been monitoring at the Sentry reports for mismatches between the app-services state machine and the android state machine. The events have gotten pretty low as a result of several fixes. They're now at about 5 per day, down from about 500 a month ago. From what I can see, the remaining events are caused by 2 cases and both of them represent issues in the android code. The first mismatch is on access token key errors. In this case [the android-components code](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/State.kt#177) transitions directly to the authentication problems state. However, [the application-services code](https://github.com/mozilla/application-services/blob/main/components/fxa-client/src/state_machine/internal_machines/connected.rs#L19) forces you to check the authorization status and only transitions to the auth issues state if that fails. The "authorization status check" means attempting to get a new access token from the refresh token. My feeling is that the android-components code is wrong since there are times where the access token has expired, but you would be able to get a new one using the refresh token. I could be wrong though, maybe the android-components code has identified cases where we know the refresh token is definitely invalid and we can skip the authentication check. However, even if that's true I think the current application-services API is better from our POV. It feels much safer to always do the check an not give the consumer an event that skips it. The worst case scenario is an extra API request to FxA, which seems fine. The second mismatch feels like a pretty bad bug in the android-components code to me. [The code creates a CoroutineContext using Executors.newSingleThreadExecutor](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt#107-111) and only calls `processQueue` inside that context to try to ensure serialized execution of the queue. However, there's an issue with that model: if a task running in that CoroutineContext calls `processQueue` again then the code will run processQueue again while the first one is still running. This is currently happening in this corner case: - firefox-android performs some FxA operation inside processQueue - All the Sentry reports I read were for completing oauth, but I think it could be any operation. - The operation fails with an auth error - Eventually, [encounteredAuthError](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt#440-450) is called which starts another processQueue run (note that the first one is still in the middle of processing) - At this point everything is very weird, the inner process queue will probably cause the state machine to transition to the auth problems state. Then the first process queue call will then continue and probable error out as well, but I suspect that there can be some subtle/weird bugs that happen in this process. To summarize, I think the state machine process has found all the bugs in the app-services code it's going to find and it's now revealing bugs in the android-components code that should be fixed. The simplest way to fix them would be to migrate to using the app-services code. The other reason I want to make the switch now is https://bugzilla.mozilla.org/show_bug.cgi?id=1887071. I believe we can fix that without too much work, but for a while I was worried that we would need to know that the client transitioned to the auth issues state in order to fix it. This makes me think there are potential bugs out there that would be easier to fix if the consumers were using our state machine rather than theirs.
I think the time has finally come to do this one for a few reasons. I've been monitoring at the Sentry reports for mismatches between the app-services state machine and the android state machine. The events have gotten pretty low as a result of several fixes. They're now at about 5 per day, down from about 500 a month ago. From what I can see, the remaining events are caused by 2 cases and both of them represent issues in the android code. The first mismatch is on access token key errors. In this case [the android-components code](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/State.kt#177) transitions directly to the authentication problems state. However, [the application-services code](https://github.com/mozilla/application-services/blob/main/components/fxa-client/src/state_machine/internal_machines/connected.rs#L19) forces you to check the authorization status and only transitions to the auth issues state if that fails. The "authorization status check" means attempting to get a new access token from the refresh token. My feeling is that the android-components code is wrong since there are times where the access token has expired, but you would be able to get a new one using the refresh token. I could be wrong though, maybe the android-components code has identified cases where we know the refresh token is definitely invalid and we can skip the authentication check. However, even if that's true I think the current application-services API is better from our POV. It feels much safer to always do the check an not give the consumer an event that skips it. The worst case scenario is an extra API request to FxA, which seems fine. The second mismatch feels like a pretty bad bug in the android-components code to me. [The code creates a CoroutineContext using Executors.newSingleThreadExecutor](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt#107-111) and only calls `processQueue` inside that context to try to ensure serialized execution of the queue. However, there's an issue with that model: if a task running in that CoroutineContext calls `processQueue` again then the code will run processQueue again while the first one is still running. This is currently happening in this corner case: - firefox-android performs some FxA operation inside processQueue - All the Sentry reports I read were for completing oauth, but I think it could be any operation. - The operation fails with an auth error - Eventually, [encounteredAuthError](https://searchfox.org/mozilla-central/rev/fb2ad9ca7150890da5cadc458acdd10c87fd9a12/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt#440-450) is called which starts another processQueue run (note that the first one is still in the middle of processing) - At this point everything is very weird, the inner process queue will probably cause the state machine to transition to the auth problems state. Then the first process queue call will then continue and probably error out as well, but I suspect that there can be some subtle/weird bugs that happen in this process. To summarize, I think the state machine process has found all the bugs in the app-services code it's going to find and it's now revealing bugs in the android-components code that should be fixed. The simplest way to fix them would be to migrate to using the app-services code. The other reason I want to make the switch now is https://bugzilla.mozilla.org/show_bug.cgi?id=1887071. I believe we can fix that without too much work, but for a while I was worried that we would need to know that the client transitioned to the auth issues state in order to fix it. This makes me think there are potential bugs out there that would be easier to fix if the consumers were using our state machine rather than theirs.