Move optional synchronous user agent fetch out of start up #1181

gdeluna-branch · 2024-03-25T23:49:59Z

Reference

SDK-XXX -- <TITLE>.

Description

Due to an external bug, an alternative to the original user agent string method was introduced to mitigate these crashes. However, the alternative method had some impact on app start up due to the limitation that it needed the main thread in order to access webview instance's settings.

To mitigate the performance impact of the workaround, the fetch routines have been refactored into coroutines to be invoked at more opportune times.

getUserAgentAsync executes immediately after the session initialization in the background.
- If a v2 event were immediately enqueued, the event will have a lock awaiting its completion
getUserAgentSync executes immediately before a v2 event is enqueued only if the public boolean Branch.setIsUserAgentSync(true); has been set

Thereafter, the result of either operation is cached in memory as was before.

Because of the queueing mechanism, regardless of the thread, the request object must still be alerted when an operation completes. This is done in the continuation from the coroutines. Regardless of the result of the fetch, either a valid string or null, the lock is removed to not dead lock the SDK.

Testing Instructions

Integrate in both main thread and backgrounded implementations. The SDK should never lock up.

Refer to logs for the following cases:

1. Sync True, Main thread

02:08:26.237 V Queue is: io.branch.referral.ServerRequestRegisterOpen@92ddee5 with locks []
02:08:26.237 V onRequestSucceeded io.branch.referral.ServerRequestRegisterOpen@92ddee5 io.branch.referral.ServerResponse@b7afecc on callback io.branch.referral.BranchUniversalReferralInitWrapper@283f6b0
02:08:26.239 D branch init complete!
02:08:26.240 V Deferring userAgent string call for sync retrieval
02:08:26.240 V onInitSessionCompleted on thread main

{send event}

02:08:43.589 V Start invoking getUserAgentSync from thread main
02:08:43.597 V Preparing V2 event, user agent is
02:08:43.597 V handleNewRequest adding process wait lock USER_AGENT_STRING_LOCK
02:08:43.598 D handleNewRequest io.branch.referral.util.BranchEvent$1@6ca2d85
02:08:43.603 V processNextQueueItem handleNewRequest
02:08:43.604 V Queue is: io.branch.referral.util.BranchEvent$1@6ca2d85 with locks [USER_AGENT_STRING_LOCK]
02:08:43.604 D processNextQueueItem, req io.branch.referral.util.BranchEvent$1@6ca2d85
02:08:43.606 V Begin getUserAgentSync Thread[main,5,main]
02:08:43.979 V End getUserAgentSync Thread[main,5,main] Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:08:43.979 V onUserAgentStringFetchFinished, releasing lock
02:08:43.979 V processNextQueueItem onUserAgentStringFetchFinished

2. Sync false, Main Thread

02:20:38.631 V onRequestSucceeded io.branch.referral.ServerRequestRegisterOpen@9f3ebc8 io.branch.referral.ServerResponse@1b41e1b on callback io.branch.referral.BranchUniversalReferralInitWrapper@b6ad24f
02:20:38.635 D branch init complete!
02:20:38.638 V onInitSessionCompleted on thread main
02:20:38.638 V Begin getUserAgentAsync Thread[DefaultDispatcher-worker-1,5,main]
02:20:38.638 V processNextQueueItem onPostExecuteInner
02:20:38.640 V Queue is:
02:20:39.009 V End getUserAgentAsync Thread[DefaultDispatcher-worker-1,5,main] Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:20:39.009 V onInitSessionCompleted resumeWith userAgent Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36 on thread DefaultDispatcher-worker-1
02:20:39.009 V processNextQueueItem getUserAgentAsync resumeWith
02:20:39.009 V Queue is:

{send event}

02:21:13.581 V userAgent was cached: Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:21:13.581 V processNextQueueItem setPostUserAgent
02:21:13.581 V Queue is:
02:21:13.585 V Preparing V2 event, user agent is Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:21:13.586 D handleNewRequest io.branch.referral.util.BranchEvent$1@25278e

3. Sync true, Background thread

02:23:14.740 V Deferring userAgent string call for sync retrieval
02:23:14.743 V onInitSessionCompleted on thread main
02:23:14.745 V processNextQueueItem onPostExecuteInner
02:23:14.746 V Queue is:

{send event}

02:23:35.130 V Start invoking getUserAgentSync from thread RxNewThreadScheduler-2
02:23:35.138 V Preparing V2 event, user agent is
02:23:35.138 V handleNewRequest adding process wait lock USER_AGENT_STRING_LOCK
02:23:35.139 D handleNewRequest io.branch.referral.util.BranchEvent$1@e38e414
02:23:35.140 V Begin getUserAgentSync Thread[main,5,main]
02:23:35.141 V processNextQueueItem handleNewRequest
02:23:35.142 V Queue is: io.branch.referral.util.BranchEvent$1@e38e414 with locks [USER_AGENT_STRING_LOCK]
02:23:35.142 D processNextQueueItem, req io.branch.referral.util.BranchEvent$1@e38e414
02:23:35.601 V End getUserAgentSync Thread[main,5,main] Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:23:35.602 V onUserAgentStringFetchFinished, releasing lock

{send event 2}

02:27:03.399 V userAgent was cached: Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:27:03.401 V processNextQueueItem setPostUserAgent
02:27:03.402 V Queue is:
02:27:03.409 V Preparing V2 event, user agent is Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:27:03.409 D handleNewRequest io.branch.referral.util.BranchEvent$1@8e0cc59
02:27:03.414 V processNextQueueItem handleNewRequest
02:27:03.415 V Queue is: io.branch.referral.util.BranchEvent$1@8e0cc59 with locks []

4. Sync false, Background thread

02:35:39.574 V onInitSessionCompleted on thread main
02:35:39.574 V Begin getUserAgentAsync Thread[DefaultDispatcher-worker-1,5,main]
02:35:39.608 V processNextQueueItem onPostExecuteInner
02:35:39.609 V Queue is:
02:35:40.041 V End getUserAgentAsync Thread[DefaultDispatcher-worker-1,5,main] Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:35:40.042 V onInitSessionCompleted resumeWith userAgent Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36 on thread DefaultDispatcher-worker-1

{send event}

02:36:06.028 V userAgent was cached: Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:36:06.028 V processNextQueueItem setPostUserAgent
02:36:06.029 V Queue is:
02:36:06.039 V Preparing V2 event, user agent is Mozilla/5.0 (Linux; Android 14; sdk_gphone64_x86_64 Build/UE1A.230829.036.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/122.0.6261.120 Mobile Safari/537.36
02:36:06.040 D handleNewRequest io.branch.referral.util.BranchEvent$1@b68c11a
02:36:06.042 V processNextQueueItem handleNewRequest
02:36:06.043 V Queue is: io.branch.referral.util.BranchEvent$1@b68c11a with locks []

Risk Assessment [`LOW`]

I, the PR creator, have tested — integration, unit, or otherwise — this code.

Reviewer Checklist (To be checked off by the reviewer only)

JIRA Ticket is referenced in PR title.
Correctness & Style
- Conforms to AOSP Style Guides
- Mission critical pieces are documented in code and out of code as needed.
Unit Tests reviewed and test issue sufficiently.
Functionality was reviewed in QA independently by another engineer on the team.

cc @BranchMetrics/saas-sdk-devs for visibility.

codecov · 2024-03-26T00:16:35Z

Codecov Report

Attention: Patch coverage is 55.22388% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 38.24%. Comparing base (1b6bb47) to head (103722e).

Files	Patch %	Lines
...K/src/main/java/io/branch/referral/DeviceInfo.java	48.64%	17 Missing and 2 partials ⚠️
...main/java/io/branch/referral/util/BranchEvent.java	53.33%	6 Missing and 1 partial ⚠️
...a/io/branch/referral/ServerRequestInitSession.java	72.72%	1 Missing and 2 partials ⚠️
...h-SDK/src/main/java/io/branch/referral/Branch.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1181      +/-   ##
============================================
+ Coverage     38.11%   38.24%   +0.13%     
+ Complexity      689      687       -2     
============================================
  Files            59       59              
  Lines          6279     6309      +30     
  Branches        939      944       +5     
============================================
+ Hits           2393     2413      +20     
- Misses         3443     3448       +5     
- Partials        443      448       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Branch-SDK/src/main/java/io/branch/coroutines/DeviceSignals.kt

echo-branch · 2024-03-26T22:05:00Z

Can we also add a performance test as well. Just to stress test the API.

NidhiDixit09

Code Looks Good to me.
I have validated all four cases Sync True/False, Main thread / Background Thread. Output in logs is macthing with the logs mentioned here.
Note: I have called Branch.setIsUserAgentSync(true); before creating Branch Instance.

gdeluna-branch · 2024-03-26T23:42:05Z

Can we also add a performance test as well. Just to stress test the API.

Thanks for the suggestion, adding tests preemptively enqueuing several events, I see the dispatcher had already enqueued each coroutine and executed them all one by one. I'll make an optimization.

(Although subsequent executions were much much faster. It's still redundant and on dispatcher main)

Move synchronous user agent fetch out of start up

a485ac0

gdeluna-branch requested a review from nsingh-branch March 25, 2024 23:51

gdeluna-branch added 4 commits March 26, 2024 02:39

move sync call to main dispatcher

13a528d

Update CustomBranchApp.java

24efdd7

Update DeviceInfo.java

82fd841

Update DeviceInfo.java

9f87b68

gdeluna-branch changed the title ~~WIP: Move optional synchronous user agent fetch out of start up~~ Move optional synchronous user agent fetch out of start up Mar 26, 2024

gdeluna-branch requested a review from NidhiDixit09 March 26, 2024 10:34

echo-branch reviewed Mar 26, 2024

View reviewed changes

Branch-SDK/src/main/java/io/branch/coroutines/DeviceSignals.kt Show resolved Hide resolved

echo-branch reviewed Mar 26, 2024

View reviewed changes

Branch-SDK/src/main/java/io/branch/coroutines/DeviceSignals.kt Show resolved Hide resolved

NidhiDixit09 approved these changes Mar 26, 2024

View reviewed changes

nsingh-branch approved these changes Mar 27, 2024

View reviewed changes

add else case to add object

103722e

echo-branch self-requested a review March 27, 2024 21:37

echo-branch approved these changes Mar 27, 2024

View reviewed changes

gdeluna-branch merged commit 72e3d1d into master Mar 27, 2024
7 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move optional synchronous user agent fetch out of start up #1181

Move optional synchronous user agent fetch out of start up #1181

gdeluna-branch commented Mar 25, 2024 •

edited

Loading

codecov bot commented Mar 26, 2024 •

edited

Loading

echo-branch commented Mar 26, 2024

NidhiDixit09 left a comment

gdeluna-branch commented Mar 26, 2024 •

edited

Loading

Move optional synchronous user agent fetch out of start up #1181

Move optional synchronous user agent fetch out of start up #1181

Conversation

gdeluna-branch commented Mar 25, 2024 • edited Loading

Reference

Description

Testing Instructions

1. Sync True, Main thread

2. Sync false, Main Thread

3. Sync true, Background thread

4. Sync false, Background thread

Risk Assessment [LOW]

Reviewer Checklist (To be checked off by the reviewer only)

codecov bot commented Mar 26, 2024 • edited Loading

Codecov Report

echo-branch commented Mar 26, 2024

NidhiDixit09 left a comment

Choose a reason for hiding this comment

gdeluna-branch commented Mar 26, 2024 • edited Loading

gdeluna-branch commented Mar 25, 2024 •

edited

Loading

Risk Assessment [`LOW`]

codecov bot commented Mar 26, 2024 •

edited

Loading

gdeluna-branch commented Mar 26, 2024 •

edited

Loading