Repository navigation
[SDK-789] fix(android): show in-app messages on the main thread and compile against native 3.10.1 - #918
Conversation
… compiles against native 3.10.1 The published iterableapi 3.10.1 AAR is compiled with Kotlin 1.9 without jvm-default, so onEmbeddedMessagingSyncSucceeded and onEmbeddedMessagingSyncFailed are abstract to Java implementers. RNIterableAPIModuleImpl did not override them, so the module failed javac. Add no-op overrides that match the native defaults.
Iterable.inAppManager.showMessage called IterableInAppManager.showMessage on the React Native native-modules thread. Since native Android 3.9.0, a host activity that is not a FragmentActivity takes the Dialog path, whose LifecycleRegistry.addObserver call throws IllegalStateException off the main thread and kills the process. - Post the native call with UiThreadUtil.runOnUiThread, matching setAutoDisplayPaused and the native iOS SDK. - Reject when the message id is missing instead of passing null into native (NPE). - Resolve null when the message is dismissed without a URL instead of NPE on toString. - Fix the empty messageId check, which used reference equality. Adds the first JVM unit tests for the Android bridge (Robolectric + Mockito, versions mirrored from iterable-android-sdk) and excludes them from the npm package.
1 new issue
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| } | ||
| }); | ||
| } | ||
| }); |
|
|
Coverage Impact This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
| public void showMessageFromBridgeThreadDisplaysOnMainThread() throws InterruptedException { | ||
| final AtomicReference<Thread> displayThread = new AtomicReference<>(); | ||
| doAnswer(invocation -> { | ||
| displayThread.set(Thread.currentThread()); | ||
| return null; | ||
| }).when(inAppManager).showMessage(eq(message), eq(true), any(IterableHelper.IterableUrlCallback.class)); | ||
|
|
||
| Thread bridgeThread = new Thread(() -> module.showMessage(MESSAGE_ID, true, promise)); | ||
| bridgeThread.start(); | ||
| bridgeThread.join(); | ||
|
|
||
| assertNull(displayThread.get()); | ||
|
|
||
| shadowOf(Looper.getMainLooper()).idle(); | ||
|
|
||
| assertSame(Looper.getMainLooper().getThread(), displayThread.get()); | ||
| } |
There was a problem hiding this comment.
What the spec says: SDK-789 is that hop. SDK-790 requires :iterable_react-native-sdk:testDebugUnitTest on pull requests to master. This PR’s GitHub Unit tests job is Jest and succeeded. ci.yml did not run.
Why it conflicts: The automated proof of the crash fix is the local Gradle command in the PR body. A later change can remove the hop and the checks that actually ran on #918 stay green. SDK-790 is the ticket that closes that gap; the test itself is the right check for this bridge.
Suggested action: Land SDK-790 so this suite runs on master pull requests before 3.2.1 is published. #918 does not need a CI change of its own.
2 participants
📝 Summary
Show in-app messages on the main thread on Android, and make the Android module compile against its pinned native SDK 3.10.1.
🎟️ Jira Ticket: SDK-789
📖 Description
1. The Android module does not compile in 3.2.0 (commit 1)
The published
iterableapi:3.10.1AAR is compiled with Kotlin 1.9 without-Xjvm-default, soIterableEmbeddedUpdateHandler.onEmbeddedMessagingSyncSucceededandonEmbeddedMessagingSyncFailedarepublic abstractto Java (javapconfirms, bodies live in$DefaultImpls).RNIterableAPIModuleImpldoes not override them, so javac fails on pristinemasterand on the 3.2.0 npm tarball. This adds no-op overrides matching the native defaults.This went unnoticed because
ci.ymltriggers onmainwhile this repo's default branch ismaster; the workflow has 0 runs ever. Tracked separately.2.
showMessagecrash (commit 2)Iterable.inAppManager.showMessagecalledIterableInAppManager.showMessageon the RN native-modules thread. Native Android has no main-thread hop on that path.FragmentActivityhost logged a warning and showed nothing.LifecycleRegistry.addObserverthrowsIllegalStateExceptionoff the main thread. The customer trace line numbers match 3.10.x exactly.FragmentActivityhosts (stockReactActivity) were not crashing:DialogFragment.show()goes throughcommit(), which only posts to the host handler. The main-thread assertion is inensureExecReady(commitNow/executePendingTransactions). That is why this has been latent since 2020.Changes in
showMessage:UiThreadUtil.runOnUiThread, same assetAutoDisplayPaused(fixed for the same reason infdf29d54) and the native iOS SDK'sshow(message:).nullwent into native and NPE'd onmessage.isJsonOnly(), the same class as SDK-785 (fix: skip setRead when the in-app message is no longer queued #916).nullwhen the message is dismissed by tapping outside. Previouslyurl.toString()NPE'd on the main thread.messageId == ""used reference equality; nowisEmpty().Known parity gap: the iOS bridge logs and never settles the promise on a missing id, Android now rejects. Follow-up ticket to make iOS reject too.
Tests
First JVM unit tests for the Android bridge: Robolectric + Mockito, versions mirrored from
iterable-android-sdk/iterableapi/build.gradle.android/src/testis excluded from the npm package.Against the old
showMessage, 3 of 5 fail (thread, missing message, null URL). With the fix 5/5 pass.🧪 How to test?
cd example/android && ./gradlew :iterable_react-native-sdk:testDebugUnitTest(JDK 17)./gradlew :app:assembleDebug -PreactNativeArchitectures=arm64-v8abuilds the example appyarn test(585 passing),yarn typecheck,yarn lintComponentActivitythat is not aFragmentActivity, callIterable.inAppManager.showMessage(message, true)from JS. Before: process dies withMethod addObserver must be called on the main thread. After: message displays.🧾 Changelog
Added under
## 3.2.1(compile fix andshowMessagecrash fix).📚 Docs PR if applicable
Follow-up:
iterable-docsdocumentsshowMessageasPromise<string | undefined>with no rejection case. Ticket to follow.Lands before #917 is published so both fixes ship in 3.2.1.