BraintreeCancelListener.onCancel() is not called

[nzakharchenko]

When I'm starting payment process on device with installed PayPal application, and after user press "Cancel" button or HW Back button - I'm not getting any notification about that. BraintreeCancelListener.onCancel() is not called like it is supposed to be.
In case when user doesn't have PayPal application and WebView started - it works correctly.

Starting payment like that:

PayPalRequest request = new PayPalRequest("1");
PayPal.requestOneTimePayment(mBraintreeFragment, request);

Configuration that I'm using:

compileSdkVersion 23
buildToolsVersion "23.0.2"
targetSdkVersion 23

compile "com.braintreepayments.api:braintree:2.0.1"

[sdcoffey]

Hey there, thanks for letting us know! I'll take a look tomorrow and get
back with you.

Steve
On Tue, Nov 24, 2015 at 14:28 nzakharchenko notifications@github.com
wrote:

When I'm starting payment process on device with installed PayPal
application, and after user press "Cancel" button or HW Back button - I'm
not getting any notification about that. BraintreeCancelListener.onCancel()
is not called like it is supposed to be.
In case when user doesn't have PayPal application and WebView started - it
works correctly.

Configuration that I'm using:

compileSdkVersion 23
buildToolsVersion "23.0.2"
targetSdkVersion 23

compile "com.braintreepayments.api:braintree:2.0.1"

—
Reply to this email directly or view it on GitHub
#63.

[sdcoffey]

Hey @nzakharchenko,

@quinnjn was able to pin down that bug this morning. We'll have a release out for this shortly. Thanks again for letting us know!

Cheers,
Steve

[nzakharchenko]

Hello @sdcoffey !

Thank you for information! Waiting for release :)

BR, Nick

[nzakharchenko]

Hello @sdcoffey, @quinnjn, Is it any update on this issue? When I could expect it would be fixed?

[vovkab]

Looks like there is a bigger problem in both v1 and v2 apis. If user cancel it will never call error or on cancel listeners.

[lkorth]

@vovkab: there was no cancel listener in v1 and canceling is not an error. It's left up to the integrator to handle the cancel in Activity#onActivityResult.

@nzakharchenko: the fix has been made and will be out in the next release. We're planning on releasing it early next week.

[nzakharchenko]

@lkorth cool! thank you for information!

[vovkab]

@lkorth I know, but this is really strange decision. Since now we need to have 2-3 listeners and also handle cancel somewhere else.

But anyway here is few bugs that is there live right now:
v1:

    public synchronized void finishPayWithPayPal(Activity activity, int resultCode, Intent data) {
        try {
            PayPalAccountBuilder payPalAccountBuilder = mBraintreeApi.handlePayPalResponse(activity,
                    resultCode, data);
            if (payPalAccountBuilder != null) {
                create(payPalAccountBuilder);
            }
        } catch (ConfigurationException e) {
            postUnrecoverableErrorToListeners(e);
        }
    }

So if for some reasons payPalAccountBuilder is null UI stuck, the flow is broken.

The same or even more in V2:

PayPal. onActivityResult:

protected static void onActivityResult(final BraintreeFragment fragment, int resultCode,
            Intent data) {
        if (resultCode == Activity.RESULT_OK && data != null) {
            try {
                boolean isAppSwitch = isAppSwitch(data);
                Result result = getResultFromIntent(fragment.getActivity(), data);
                ResultType resultType = result.getResultType();
                switch (resultType) {
                    case Error:
                        sendAnalyticsEventForSwitchResult(fragment, isAppSwitch, "failed");
                        break;
                    case Cancel:
                        onCancel(fragment, result, isAppSwitch);
                        break;
                    case Success:
                        onSuccess(fragment, data, result);
                        break;
                }
            } catch (InvalidArgumentException error) {
                fragment.postCallback(error);
            }
        }
    }

1. if data is null - flow is broken. Clients have no idea what library uses internally that is why we have listeners. Clients should not care if data null.
2. Cancel also is not handled. Why have cancel listener and not send cancel events?
3. Maybe I'm missing something here, but I don't see sendAnalyticsEventForSwitchResult sending error back to listeners.
4. There is also no default switch case, so if Paypal changes ResultType our app will be broken.

p.s. Maybe there is a chance you could make library with a blocked calls, so we can build something on top of it, for example we could use RxJava and have just one onActivityResult blocking call to get result. This way we can avoid all this callback hell.

[lkorth]

The only time paypalAccountBuilder would be null is if resultCode was cancel in which case I wouldn't expect Braintree#finishPayWithPayPal to be called.

Changes have already been made for v2 and will be released early next week.

[vovkab]

Thanks @lkorth for a quick answer.

About paypalAccountBuilder - this is not true.
Let's look inside what you do in getBuilderFromActivity:

                try {
                    paypalTouchResponse = paypalTouchConfirmation
                            .toJSONObject()
                            .getJSONObject("response");
                } catch (JSONException ignored) {
                    return null;
                }

As you can see if something wrong with json, it is broken again, it will return null, and we will never get error notification.

[lkorth]

In that case, there would have been errors before this point in the PayPal SDK and it would have never gotten this far without a response.

We'll change it to return an error whenever paypalAccountBuilder is null.

[vovkab]

@lkorth thanks!

p.s. if there were more errors and you never get to this code, why do you need to return null or even check for a null? ;)

[lkorth]

@nzakharchenko the fix for this went out in 2.1.0. Let us know if you have any other problems.

@vovkab the fix on 1.x will be going out shortly.