Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

camera no longer captures images after the method confirmSetupIntent is called #93

Open
carlosanjos opened this issue Jan 8, 2020 · 13 comments

Comments

@carlosanjos
Copy link

Which platform(s) does your issue occur on?

  • Android 9.0
  • Galaxy S8+

Please, provide the following version numbers that your issue occurs with:

  • tns --version 6.3.0
  • Android 9
  • Galaxy S8+
  • macOS 10.14.5
  • "nativescript-camera": "^4.5.0"
  • "nativescript-stripe": "~6.2.1"

Please, tell us how to recreate the issue in as much detail as possible.

  1. Tap on the button to take a photo a capture any photo; Notice the Image widget will be populated with the photo just taken

  2. Fill in with a valid stripe testing card information, you can find them here https://stripe.com/docs/testing

  3. Tap on save payment card button

  4. A pop-up will be displayed informing the operation succeeded

  5. Refer to step one and take another photo; Notice the photo won't be captured this time

Is there any code involved?

@RobertGardner
Copy link
Collaborator

Sorry, I don't have my laptop to reproduce this or confirm the problem, but I'm suspicious it's because you are calling alert() asynchronously.

In ItemsComponent.confirm you display an alert after receiving a response from confirmSetupIntent. This call is being made asynchronously since it's in the Observer's subscribe method. Therefore, the UI state may not be consistent and either the Stripe service or the Camera service is getting confused.

You could try setting a timer instead of displaying the alert directly, and then display the alert once everything has settled down.

@carlosanjos
Copy link
Author

Actually I have the alert() call only to have some visual cue. My initial example I was printing to the console. I'll will update the example accordingly

@luke-hawk
Copy link

luke-hawk commented May 19, 2020

I'm facing the same issue with the standard implementation! @carlosanjos Did you find a workaround?

@carlosanjos
Copy link
Author

@luke-hawk I changed my flow so camera and payment cards wouldn't mix. It's been a while, but according to my notes, the lines where const activity = androidApp.foregroundActivity; in nativescript-stripe happens, the camera activity fails

@luke-hawk
Copy link

@carlosanjos Cheers! Yep, I've already suspected that foregroundActivity is causing the issue. Unfortunately I can't really change my flow. Hope to find another way to fix it. Do you know whether it's possible to remove stripe from foregroundActivity ?

@luke-hawk
Copy link

luke-hawk commented May 19, 2020

@RobertGardner I think I found the cause for this bug which seems to have it's root here:

private patchActivity(): android.app.Activity {
let activity = androidApp.foregroundActivity;
let session = this;
// TODO: How do I call the callback? The code below doesn't work,
// it throws an "undefined" exception. Note JSON.stringify(oldResultCallback) returns "undefined".
let oldResultCallback = activity.onActivityResult;
console.log("oldResultCallback: " + JSON.stringify(oldResultCallback));
activity.onActivityResult = function (requestCode, resultCode, data) {
// if (oldResultCallback) oldResultCallback(requestCode, resultCode, data);
session.native.handlePaymentData(requestCode, resultCode, data);
};
let oldDestroyCallback = activity.onDestroy;
console.log("oldDestroyCallback: " + JSON.stringify(oldDestroyCallback));
activity.onDestroy = function () {
// if (oldDestroyCallback) oldDestroyCallback();
session.native.onDestroy();
getLocalBroadcastManagerPackage().LocalBroadcastManager.getInstance(activity).unregisterReceiver(this.receiver);
};
return activity;
}
}

...more specifically, the onActivityResult callback seems to be hijacking similar callbacks for other libraries as well. Adding a console.log to following lines is also logging when using nativescript-camera and nativescript-imagepicker.

activity.onActivityResult = function (requestCode, resultCode, data) { 
   console.log('onActivityResult from nativescript-stripe')
    // if (oldResultCallback) oldResultCallback(requestCode, resultCode, data); 
     session.native.handlePaymentData(requestCode, resultCode, data); 
 }; 

...Also, when completely commenting out the above mentioned function, nativescript-camera and nativescript-imagepicker work as expected. Any idea what we can do about it?

P.S.: These seem to be the relavant snippets for nativescript-camera

appModule.android.foregroundActivity.startActivityForResult(takePictureIntent, REQUEST_IMAGE_CAPTURE);

and nativescript-imagepicker

application.android.foregroundActivity.startActivityForResult(intent, RESULT_CODE_PICKER_IMAGES);

@RobertGardner
Copy link
Collaborator

Unfortunately, that code is necessary for the "standard" charge flow to work. It's how messages pass between the Stripe SDK and the client.

I'm not an Android developer, so I'm not sure if there's a better way to do activity result chaining. I had written code trying to do the chaining, but when I call activity.onActivityResult it returns undefined. (That is why the code calling the old callback is commented out.) If someone knows the proper way to get the old callback so I can properly chain, I can fix this quite easily. Or maybe it will return a valid callback in some situations like with the camera and I just need to uncomment that line? If you could check it with a debugger that would be helpful. You can play around with fixing it in the JS code in your app by making changes, deleting your platforms folder and rebuilding. (Don't delete node_modules. :-) )

Also, Stripe completely changed the way the activity result callback works in their Android v12.8 release. It completely eliminates this callback and the need to use an activity at all. I'm sure it would fix the problem. Unfortunately, due to the {NS} bugs with their Kotlin integration (see NativeScript/android#1604), I'm not able to upgrade to the v12.8 release until that's fixed. There hasn't been any activity on that bug since I filed it almost a month ago. Maybe people can put some pressure on {NS} to work on it?

@luke-hawk
Copy link

luke-hawk commented May 19, 2020

@RobertGardner Hm. I see! Any idea how I can remove / reset activity.onActivityResult then? I could then at least find a work around and remove the listener when my component where the stripe checkout takes place gets destroyed. I also found out that:

application_1.android.on('activityResult', (args) => {
  session.native.handlePaymentData(args.requestCode, args.resultCode, args.intent);
})

does the same job. (But also causes the same bug)

@luke-hawk
Copy link

luke-hawk commented May 20, 2020

@RobertGardner Would you mind if we chance the before mentioned callback to an event listener of the following kind:

application_1.android.on('activityResult', (args) => {
        if (args.requestCode === 6000 && args.resultCode === android.app.Activity.RESULT_OK) {
            session.native.handlePaymentData(args.requestCode, args.resultCode, args.intent);
        }
})

This way I can unmount the event listener when not needed and checking for the requestCode prevents from crashing if the same callback is fired from other plugins.

I'd be glad to add a PR for this if you're fine with changing it this way.

@RobertGardner
Copy link
Collaborator

I did some debugging to see if I could figure out what's going on. The activity result handler is registered in the PaymentSession constructor. So every time you create a new PaymentSession you will register the handler.

The activity handler is removed (as best as I can tell -- I believe it's actually the Stripe SDK that removes it) in activity.onDestroy().

I am getting the current activity by calling androidApp.foregroundActivity, which is the only way I can figure out how to access the activity. However, activity.onDestroy() is only being called when the demo application deactivates (using, for example, the back button from the home page).

This suggests that the foreground activity is the home page. I tried waiting until the page containing the payment form was completely drawn before creating PaymentSession but the foregroundActivity was the same and onDestroy still wasn't called until app deactivation.

There are some possible solutions I could recommend until I'm able to get rid of all this activity registration stuff. One is that you figure out how to create a foreground activity that is created and destroyed on your Stripe form page. The other is that I expose the register/unregister functions so you can call them directly. Let me know how you'd like to proceed.

Note that while debugging I discovered a bug in my onDestroy implementation and it crashes when it is executed. I will fix that, but thought I'd wait to see if you want me to expose those register/unregister functions.

@luke-hawk
Copy link

luke-hawk commented May 20, 2020

@RobertGardner Ok, sounds like good! I would prefer the option to register/unregister manually – this is exactly what I was looking for all the time and would suit my integration nicely :-) ...let me know if you need any help! Thanks a lot :-)

@RobertGardner
Copy link
Collaborator

I submitted the change to the ondestroy branch. Please take a look.

The technique I'm using is replacing the Activity.onActivityResult and onDestroy callbacks with my own callbacks in registerActivityResult, then restoring the old versions in unregisterActivityResult. Your technique of registering event listeners might be a better choice, because it seems less intrusive. I could imagine scenarios where replacing the overloaded methods could cause problems. However, I have no way of testing this. If you think event listeners is better, please send me a PR or the code to use. Thanks.

@luke-hawk
Copy link

Ok, I'm quite busy at the moment but I will have a look at it next week for sure and will get back to you then. Cheers!

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

No branches or pull requests

3 participants