fix(android): let the authorization response survive a task reset (#1094) - #1137
fix(android): let the authorization response survive a task reset (#1094)#1137dparcerisa wants to merge 1 commit into
Conversation
Closes the "Data intent is null" failure reported in FormidableLabs#1094: start an authorization, leave the app (to read a one-time code from a mail app, for instance), come back from the launcher icon, and the flow dies. Two things combine to cause it. AppAuth keeps the pending request inside AuthorizationManagementActivity, which lives in the caller's task; when the host app's launcher activity is launchMode="singleTask" — what React Native recommends for deep linking — re-entering from the icon clears the task above the root and destroys it, and AppAuth then logs "No stored state - unable to handle response". Separately, the response is delivered with startActivityForResult, which is bound to the living activity instance and cannot cross tasks, so it arrives as a null Intent. This change addresses the delivery half: the response now comes back through a PendingIntent addressed to the launcher activity, completed from onNewIntent (declared but empty until now). The result handling is factored into a shared method so the activity-result path is unchanged. Host apps also need AuthorizationManagementActivity to survive, which they can do by giving it its own taskAffinity and singleInstance without touching their launcher activity's launchMode. Both halves are needed; neither works alone. Measured on an Android 15 emulator. The workaround suggested in FormidableLabs#1094 — switching the launcher activity to singleTop — was also tried and is worse: the error disappears but the promise never settles, so the login silently hangs. android:alwaysRetainTaskState="true" has no effect.
|
@dparcerisa is attempting to deploy a commit to the Nearform Team on Vercel. A member of the Team first needs to authorize it. |
|
|
Closing this, and I want to be straight about why: we shipped it, QA broke it, and it is worse than what it replaces. Two findings, both from real devices rather than emulators. 1. It trades a catchable error for silence. Delivering the response through a
So the change does not fix the bug on the devices most likely to hit it; it converts 2. The host-app half I recommended breaks retrying. Marking What we did instead, in case it is useful to anyone landing here from #1094: we left the library alone and handled it in JS. Remember that a flow is in the air, clear it on every settled path, and if either Sorry for the noise. I would rather withdraw this than leave a proposal standing that I now know regresses real devices. |
Draft, because the last section is a real design question I would rather have answered before polishing this.
Fixes the
Data intent is nullfailure in #1094: start an authorization, leave the app to read a one-time code from a mail app, come back from the launcher icon, and the login dies. Anyone whose provider emails a code hits it.Two causes, and neither fix works alone
AuthorizationManagementActivity, which lives in the caller's task. When the host app's launcher activity islaunchMode="singleTask"— what React Native recommends for deep linking — re-entering from the icon clears the task above the root and destroys it. Even a perfectly delivered response has nothing to match against.authorizeWithConfigurationdelivers viastartActivityForResult(authIntent, 52), which is bound to the living activity instance and cannot cross tasks. Once the activity is gone the result arrives as a null Intent.What this PR changes
Only the delivery half. The response now comes back through a
PendingIntentaddressed to the launcher activity — the component only, no MAIN/LAUNCHER categories, since those are what trigger the reset — and is completed fromonNewIntent, which the module declared but left empty. Result handling is factored into a shared method, so the activity-result path is untouched.The other half is on the host app: give
AuthorizationManagementActivityits owntaskAffinityplussingleInstance, which keeps the launcher activity'slaunchModealone. Happy to document that in the README if this direction is accepted.Measured, on an Android 15 emulator
singleTopon the launcher activity (the workaround in #1094)android:alwaysRetainTaskState="true"singleTaskclear-topAlso checked that deep links and universal links still resume a single launcher-activity instance rather than spawning a second.
The open question
This changes how the response arrives for every Android consumer. It could instead sit behind a config flag (
androidUsePendingIntentDeliveryor similar), at the cost of a JS-side option and two code paths to maintain.I lean unconditional — the
PendingIntentpath is strictly more robust andonNewIntentis already wired throughActivityEventListener, so RN apps route it without extra work — but it is your call, and I will rework it either way. There are no tests in this area yet; tell me the shape you want and I will add them.