Skip to content

Fix push tokenization promise never settling in onActivityResult - #70

Merged
JakubKorytko merged 3 commits into
Expensify:mainfrom
MegaManSec:pr/jr-3-a330c1e
Oct 9, 2026
Merged

JakubKorytko merged 3 commits into
Expensify:mainfrom
MegaManSec:pr/jr-3-a330c1e

Conversation

@MegaManSec

@MegaManSec MegaManSec commented Feb 3, 2026 •

Copy link
Copy Markdown
Contributor

onActivityResult for push tokenization never settled the promise when resultCode was RESULT_OK with null data, or anything other than RESULT_OK/RESULT_CANCELED, so addCardToGoogleWallet/resumeAddCardToGoogleWallet hung forever.

  • RESULT_OK resolves SUCCESS regardless of data; tokenId is null when missing instead of the string "null".
  • Any other result code resolves ERROR.

… promise is not cleared

(cherry picked from commit a330c1ee2261f192103ac4e7a2fa5d86e2c59b7f)
@JakubKorytko

Copy link
Copy Markdown
Member

@MegaManSec may you resolve conflicts with main please?

@MegaManSec

Copy link
Copy Markdown
Contributor Author

@JakubKorytko done

@JakubKorytko JakubKorytko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on an emulator by calling onActivityResult directly with a fake promise:

  • RESULT_OK + null data: rejects (hangs on main)
  • unknown resultCode: rejects (hangs on main)
  • RESULT_CANCELED: resolves CANCELED, same as main

Comment thread android/src/main/java/com/expensify/wallet/WalletModule.kt Outdated
Comment thread android/src/main/java/com/expensify/wallet/WalletModule.kt Outdated
Comment thread android/src/main/java/com/expensify/wallet/WalletModule.kt Outdated
@MegaManSec MegaManSec changed the title onActivityResult can hang forever if OK but data == null, and pending… Fix push tokenization promise never settling in onActivityResult Oct 9, 2026

@JakubKorytko JakubKorytko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, all three points are covered. Re-tested, LGTM

@JakubKorytko
JakubKorytko merged commit b385fd3 into Expensify:main Oct 9, 2026
7 of 8 checks passed
@JakubKorytko

Copy link
Copy Markdown
Member

thanks

@os-botify

os-botify Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Published to npm in 0.1.40 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants