-
Notifications
You must be signed in to change notification settings - Fork 144
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
Comments
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 :( |
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. |
Cool :-) Thanks for the feedback. |
@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. |
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 :
What I want to do :
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 detected5/ 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 :
Implementation 1 : GDPR unfriendly implementation
First implementation is a "simple" call to
initSession()
:Here are results for code above :
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ application started !
In that case, it looks like we're close to what we want but :
Implementation 2 : GDPR friendly implementation
I simply added a
Branch.disableTracking(true)
call onceinitSession()
was done.Here are results for code above :
✅ application started !
✅ application started !
initSession()
:{"error":"Trouble initializing Branch. Tracking is disabled. Requested operation cannot be completed when tracking is disabled"}
❌ application not started !
initSession()
:{"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
✅ application started !
✅ application started !
initSession()
:{"error":"Trouble initializing Branch. Tracking is disabled. Requested operation cannot be completed when tracking is disabled"}
❌ application not started !
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 :initSession()
without having clicked any Branch link. It reminds me Warning. Session initialization already happened. To force a new session, set intent extra, "branch_force_new_session", to true. #653disableTracking(true)
has been called once, we cannot callinitSession()
without having clicked any Branch link, as if disabled tracking was preventinginitSession()
to be called properly.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 :
✅ application started !
✅ application started !
initSession()
:{"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
initSession()
:{"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
✅ application started !
✅ application started !
initSession()
:{"error":"User tracking is disabled and the request is not allowed"}
❌ application not started !
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 callinitSession()
without having clicked any Branch link, as if disabled tracking was preventinginitSession()
to be called properly.Implementation 4 : #653 fix + Enforcing tracking prior to
initSession()
+ being GDPR friendly implementation afterinitSession()
For this, I changed a little bit my implementation by calling
Branch.disableTracking(false)
beforeinitSession()
so that we're sure tracking is enabled and try to workaround weird error aboveResults :
✅ application started !
✅ application started !
✅ application started !
❌ 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...✅ application started !
✅ application started !
✅ application started !
❌ 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 callinginitSession()
, particularly on iOS where it looks likedisableTracking(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()
+ delayinginitSession()
+ being GDPR friendly implementation afterinitSession()
Results :
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ application started !
✅ 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 :
disableTracking()
return apromise
instead of nothing (so that I don't need a weirdsetTimeout()
)initSession()
while, on a previous app execution, we setdisableTracking(true)
The text was updated successfully, but these errors were encountered: