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

disableTracking() / initSession() error on subsequent calls #708

Open
fcamblor opened this issue Jun 14, 2022 · 4 comments
Open

disableTracking() / initSession() error on subsequent calls #708

fcamblor opened this issue Jun 14, 2022 · 4 comments

Comments

@fcamblor
Copy link

fcamblor commented Jun 14, 2022

Hello there,

I faced a lot of problems trying to make this plugin's v5.1.0 working on both iOS and Android.

My environment info :

Cordova: 10.0.0
cordova-android: 10.1.1
cordova-ios: 6.2.0
branch-cordova-sdk: 5.1.0

node: 15.14.0
npm: 7.7.6

What I want to do :

  • disable tracking for GDPR considerations
  • init Branch session on application startup
  • when branch link has been clicked, show an alert

To validate this, I was running following scenarios (both on Android & iOS) :
1/ Open Branch link
2/ Get redirected on the store for the first time as the app was not installed on my system yet
3/ Install the app through cordova (not through the store) in order to be able to debug it easily
4/ Run the app - expected to have an alert telling that branchio's +clicked_branch_link was detected
5/ Kill the app
6/ Open the app from the system directly (not through branch' link) - expected to not have any alert (no branch link clicked)
7/ Kill the app
8/ Open Branch link again - expected it to open the app and have the alert
9/ Kill the app
10/ Open the app from the system directly (not through branch' link) - expected to not have any alert (no branch link clicked)

As a backbone on all my tests, I had following code :

if (window.cordova) {
    document.addEventListener("deviceready", onDeviceReady, false);
} else {
    onDeviceReady();
}

async function onDeviceReady() {
    await Promise.all([
        initBranch()
    ])
    console.log("Application started !);
}

async function initBranch() {
    // Implementation here will vary
}

Implementation 1 : GDPR unfriendly implementation

First implementation is a "simple" call to initSession() :

async function initBranch() {
    // Retrieving Branch params
    const params = await Branch.initSession();

    console.debug(`branchio initSession() done ! => ${JSON.stringify(data)}`)
    if (params['+clicked_branch_link']) {
        // read deep link data on click
        alert(`BRANCHIO clicked link detected: ${JSON.stringify(params)}`)
    }
}

Here are results for code above :

Expectation Step Android iOS
4/ first run after installation ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
6/ Open from system ✅ No Alert shown
✅ application started !
✅ No Alert shown
✅ application started !
8/ Open (again) from branch link ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
10/ Open (again) from system ❌ Alert shown again
✅ application started !
✅ No Alert shown
✅ application started !

In that case, it looks like we're close to what we want but :

  • we have a GDPR-unfriendly implementation
  • some residual state seem to remain on Android implementation on step 10

Implementation 2 : GDPR friendly implementation

I simply added a Branch.disableTracking(true) call once initSession() was done.

async function initBranch() {
    // Retrieving Branch params
    const params = await Branch.initSession();

    // Let's configure branch to be GDPR friendly
    Branch.disableTracking(true)

    console.debug(`branchio initSession() done ! => ${JSON.stringify(data)}`)
    if (params['+clicked_branch_link']) {
        // read deep link data on click
        alert(`BRANCHIO clicked link detected: ${JSON.stringify(params)}`)
    }
}

Here are results for code above :

Expectation Step Android iOS
4/ first run after installation ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
6/ Open from system ❌ got a runtime error during initSession() : {"error":"Trouble initializing Branch. Tracking is disabled. Requested operation cannot be completed when tracking is disabled"}
❌ application not started !
❌ got a runtime error during initSession() : {"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
8/ Open (again) from branch link ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
10/ Open (again) from system ❌ got a runtime error during initSession() : {"error":"Trouble initializing Branch. Tracking is disabled. Requested operation cannot be completed when tracking is disabled"}
❌ application not started !
❌ got a runtime error during initSession() : {"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !

With the introduction of disableTracking(true) call, we can see 2 different errors :

Implementation 3 : #653 fix + GDPR friendly implementation

This implementation is exactly the same than the one above, except that I applied this patch (found in #653 comments) on the Android plugin implementation to fix error pinpointed previously.

Here are results :

Expectation Step Android iOS
4/ first run after installation ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
6/ Open from system ❌ got a runtime error during initSession() : {"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
❌ got a runtime error during initSession() : {"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
8/ Open (again) from branch link ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
10/ Open (again) from system ❌ got a runtime error during initSession() : {"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
❌ got a runtime error during initSession() : {"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !

By providing the fix for #653, we can now have an homogeneous behaviour between both iOS and Android, where it looks like that when disableTracking(true) has been called once, we cannot call initSession() without having clicked any Branch link, as if disabled tracking was preventing initSession() to be called properly.

Implementation 4 : #653 fix + Enforcing tracking prior to initSession() + being GDPR friendly implementation after initSession()

For this, I changed a little bit my implementation by calling Branch.disableTracking(false) before initSession() so that we're sure tracking is enabled and try to workaround weird error above

async function initBranch() {
    // Enforcing tracking is enabled to avoid "User tracking is disabled and the request is not allowed" error
    Branch.disableTracking(false)

    // Retrieving Branch params
    const params = await Branch.initSession();

    // Let's configure branch to be GDPR friendly
    Branch.disableTracking(true)

    console.debug(`branchio initSession() done ! => ${JSON.stringify(data)}`)
    if (params['+clicked_branch_link']) {
        // read deep link data on click
        alert(`BRANCHIO clicked link detected: ${JSON.stringify(params)}`)
    }
}

Results :

Expectation Step Android iOS
4/ first run after installation ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
6/ Open from system ✅ No Alert shown
✅ application started !
✅ No Alert shown
❌ application not started due to timeout occured during initSession(). If I restart the app, the app is then ✅ well started (no timeout). And if I restart the app again, I get the timeout again etc...
8/ Open (again) from branch link ❌ No Alert shown
✅ application started !
✅ Alert shown
✅ application started !
10/ Open (again) from system ✅ No Alert shown
✅ application started !
✅ No Alert shown
❌ application not started due to timeout occured during initSession(). If I restart the app, the app is then ✅ well started (no timeout). And if I restart the app again, I get the timeout again etc...

That's better, but it seems like disableTracking(false) is not always taken into consideration when calling initSession(), particularly on iOS where it looks like disableTracking(false) is randomly applied.

Let's see if adding some delay after disableTracking(false) will help...

Implementation 5 : #653 fix + Enforcing tracking prior to initSession() + delaying initSession() + being GDPR friendly implementation after initSession()

async function initBranch() {
    // Enforcing tracking is enabled to avoid "User tracking is disabled and the request is not allowed" error
    Branch.disableTracking(false)

    // Delaying initSession() a little bit after disableTracking(false) so that it gets taken into consideration
    await new Promise(resolve => setTimeout(resolve, 500))

    // Retrieving Branch params
    const params = await Branch.initSession();

    // Let's configure branch to be GDPR friendly
    Branch.disableTracking(true)

    console.debug(`branchio initSession() done ! => ${JSON.stringify(data)}`)
    if (params['+clicked_branch_link']) {
        // read deep link data on click
        alert(`BRANCHIO clicked link detected: ${JSON.stringify(params)}`)
    }
}

Results :

Expectation Step Android iOS
4/ first run after installation ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
6/ Open from system ✅ No Alert shown
✅ application started !
✅ No Alert shown
✅ application started !
8/ Open (again) from branch link ✅ Alert shown
✅ application started !
✅ Alert shown
✅ application started !
10/ Open (again) from system ❌ Alert shown
✅ application started !
✅ No Alert shown
✅ application started !

Looks like I'm back to Implementation 1, but while being more GDPR friendly this time.
I assume there is a bug somewhere on the subsequent calls of the Android impl (already pinpointed in "simple" Implementation 1)

Hope this whole report will help to build some improvements.
Some thoughts :

  • I assume it would be nice to make disableTracking() return a promise instead of nothing (so that I don't need a weird setTimeout())
  • I assume that there is something weird when we call initSession() while, on a previous app execution, we set disableTracking(true)
  • It looks like some state remains on Android when a click has been detected a second+ time, and the app is started again without any click on the deep link.
@fcamblor
Copy link
Author

fcamblor commented Jul 8, 2022

Do we still have people from Branchmetrics maintaining this plugin ?

Not having even a reaction on this issue in near than one month makes me wonder :(

@gdeluna-branch
Copy link
Contributor

Apologies for the lack of response @fcamblor , this one slipped me. I've gone over your write up and appreciate the details you've put in. Right now we're designing some proper fixes to the session handling for our Javascript based plugins- this is helpful data. Will keep you posted.

@fcamblor
Copy link
Author

Cool :-)

Thanks for the feedback.

@barillax
Copy link

@gdeluna-branch Is the workaround provided by @fcamblor (including the session bug patch from #653) still necessary for GDPR compliance when using the Cordova SDK? We've just submitted an initial integration for app store review using version 5.2.0, but now I'm wondering if we need to integrate these changes to avoid being flagged in the future.

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