Skip to content

fix: add check to avoid crashes when currentActivity is null - #846

Merged
charliecruzan-stripe merged 8 commits into
masterfrom
charliecruzan-currentActivity-nullsafety
Apr 18, 2022
Merged

charliecruzan-stripe merged 8 commits into
masterfrom
charliecruzan-currentActivity-nullsafety

Conversation

@charliecruzan-stripe

@charliecruzan-stripe charliecruzan-stripe commented Mar 15, 2022 •

Copy link
Copy Markdown
Collaborator

Summary

We weren't always appropriately null-checking currentActivity, and the typecast could result in crashes. We now resolve with a consistent and useful error telling the user they can retry the method.

Motivation

closes #844 and I'm sure other instances of crashes on Android
closes #881

Testing

  • I tested this manually
  • I added automated tests

Documentation

Select one:

  • I have added relevant documentation for my changes.
  • This PR does not result in any developer-facing changes.

Comment thread android/src/main/java/com/reactnativestripesdk/StripeSdkModule.kt Outdated
override fun onReceive(context: Context?, intent: Intent) {
if (intent.action == ON_GOOGLE_PAY_FRAGMENT_CREATED) {
googlePayFragment = (currentActivity as AppCompatActivity).supportFragmentManager.findFragmentByTag("google_pay_launch_fragment") as GooglePayFragment
(currentActivity as? AppCompatActivity)?.let {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was really surprised that you have to check the current activity for null everywhere, even from @ReactMethod, but I didn't really see another way.

In order to prevent this type of error in the future I would recommend:

class StripeSdkModule ...{
...
  // Set currentActivity so it uses this instead of the base class `getCurrentActivity()`
  // this will give you type safety checking and convert it to an appCompat activity
  val currentActivity: AppCompatActivity?
    get() = (super.getCurrentActivity() as? AppCompatActivity)
...
    private val googlePayReceiver: BroadcastReceiver = object : BroadcastReceiver() {
        override fun onReceive(context: Context?, intent: Intent) {
          if (intent.action == ON_GOOGLE_PAY_FRAGMENT_CREATED) {
            currentActivity?.let {
...
}

It would also be nice to see some enforcement in the code that if you can't get the appCompat activity that a promise is fired to return a result.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

thanks for the feedback! created a helper method for getting the activity or resolving the promise. It's not the perfect solution since someone can still call currentActivity, but it does avoid a lot of repeated code and hopefully anyone working here would see the current pattern for getting the activity 🤞

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@michelleb-stripe do you approve the most recent changes? I'll need to rebase but since #879 contains substantial changes to the same file, i'd rather wait till after that lands to rebase (to avoid duplicating the work)

@charliecruzan-stripe

Copy link
Copy Markdown
Collaborator Author

closes #881

@charliecruzan-stripe
charliecruzan-stripe force-pushed the charliecruzan-currentActivity-nullsafety branch from d11edcb to 369587b Compare April 18, 2022 14:18
@charliecruzan-stripe
charliecruzan-stripe force-pushed the charliecruzan-currentActivity-nullsafety branch from 369587b to 39a63e1 Compare April 18, 2022 16:26
@charliecruzan-stripe
charliecruzan-stripe merged commit e27d3f0 into master Apr 18, 2022
@charliecruzan-stripe
charliecruzan-stripe deleted the charliecruzan-currentActivity-nullsafety branch April 18, 2022 19:54
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.

null cannot be cast to non-null type androidx.appcompat.app.AppCompatActivity initGooglePay have the same init error that INIT_PAYMENTSHEET had

3 participants