-
Notifications
You must be signed in to change notification settings - Fork 40
[SDK-196] Fix retryInterval not being able to be updated #786
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
base: jwt/master
Are you sure you want to change the base?
[SDK-196] Fix retryInterval not being able to be updated #786
Conversation
|
Diff Coverage: Not applicable. There was no coverage data reported for the files in this diff. Total Coverage: This PR will not change total coverage. 🛟 Help
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
4 new issues
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
…y updated on re-initialization
…ODO for root cause resolution
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR aims to fix an issue where the retryInterval configuration was not being properly updated on Android. The fix introduces a reflection-based workaround to update the retry policy on existing authManager instances after re-initialization. However, the PR description mentions converting seconds to milliseconds, which is not evident in the actual code changes.
Key Changes:
- Added reflection code to access and update package-private
authRetryPolicyfield inIterableAuthManager - Updated example app to test retry behavior with logging and adjusted initialization flow
- Updated CHANGELOG to document the fix
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java | Added reflection-based workaround to update retry policy on existing authManager in both initialization methods |
| example/src/hooks/useIterableApp.tsx | Modified example app to test retry behavior: added timestamp tracking in authHandler, changed retryInterval value, and updated initialization flow |
| CHANGELOG.md | Added entry for version 2.2.0-alpha.2 documenting the retryInterval fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| // Use reflection to access package-private fields and methods | ||
| java.lang.reflect.Field configRetryPolicyField = config.getClass().getDeclaredField("retryPolicy"); | ||
| configRetryPolicyField.setAccessible(true); | ||
| Object retryPolicy = configRetryPolicyField.get(config); | ||
|
|
||
| if (retryPolicy != null) { | ||
| java.lang.reflect.Method getAuthManagerMethod = IterableApi.getInstance().getClass().getDeclaredMethod("getAuthManager"); | ||
| getAuthManagerMethod.setAccessible(true); | ||
| IterableAuthManager authManager = (IterableAuthManager) getAuthManagerMethod.invoke(IterableApi.getInstance()); | ||
|
|
||
| if (authManager != null) { | ||
| // Update the retry policy field on the authManager | ||
| java.lang.reflect.Field authRetryPolicyField = authManager.getClass().getDeclaredField("authRetryPolicy"); | ||
| authRetryPolicyField.setAccessible(true); | ||
| authRetryPolicyField.set(authManager, retryPolicy); | ||
| IterableLogger.d(TAG, "Updated retry policy on existing authManager"); | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, "Failed to update retry policy: " + e.getMessage()); | ||
| } |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code in this method (lines 162-183) is an exact duplicate of the reflection logic in initializeWithApiKey (lines 99-120). This violates the DRY (Don't Repeat Yourself) principle and makes maintenance harder.
Consider extracting this logic into a private helper method that can be called from both initialization methods:
private void updateAuthManagerRetryPolicy(IterableConfig config) {
// Move the try-catch block here
}
example/src/hooks/useIterableApp.tsx
Outdated
| config.retryPolicy = { | ||
| maxRetry: 5, | ||
| retryInterval: 10, | ||
| retryInterval:5, |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after colon. Should be retryInterval: 5 for consistency with the code style.
| retryInterval:5, | |
| retryInterval: 5, |
| across application | ||
|
|
||
| ### Fixes | ||
| * Fixed Android `retryInterval` not being updated on re-initialization. |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changelog entry states that retryInterval is "not being updated on re-initialization," but based on the PR description, the actual fix is about converting seconds to milliseconds. However, this conversion is missing from the code changes in Serialization.java. The changelog should more accurately describe what was actually fixed (the workaround to update retry policy on existing authManager).
| * Fixed Android `retryInterval` not being updated on re-initialization. | |
| * Fixed Android `retryInterval` value to be correctly converted from seconds to milliseconds and added workaround to update retry policy on existing authManager. |
| // Update retry policy on existing authManager if it was already created | ||
| // This fixes the issue where retryInterval is not respected after | ||
| // re-initialization | ||
| // TODO [SDK-197]: Fix the root cause of this issue, instead of this hack | ||
| try { | ||
| // Use reflection to access package-private fields and methods | ||
| java.lang.reflect.Field configRetryPolicyField = config.getClass().getDeclaredField("retryPolicy"); | ||
| configRetryPolicyField.setAccessible(true); | ||
| Object retryPolicy = configRetryPolicyField.get(config); | ||
|
|
||
| if (retryPolicy != null) { | ||
| java.lang.reflect.Method getAuthManagerMethod = IterableApi.getInstance().getClass().getDeclaredMethod("getAuthManager"); | ||
| getAuthManagerMethod.setAccessible(true); | ||
| IterableAuthManager authManager = (IterableAuthManager) getAuthManagerMethod.invoke(IterableApi.getInstance()); | ||
|
|
||
| if (authManager != null) { | ||
| // Update the retry policy field on the authManager | ||
| java.lang.reflect.Field authRetryPolicyField = authManager.getClass().getDeclaredField("authRetryPolicy"); | ||
| authRetryPolicyField.setAccessible(true); | ||
| authRetryPolicyField.set(authManager, retryPolicy); | ||
| IterableLogger.d(TAG, "Updated retry policy on existing authManager"); | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, "Failed to update retry policy: " + e.getMessage()); | ||
| } | ||
|
|
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using reflection to access package-private fields and methods is fragile and will break if the internal structure of the Iterable SDK changes. This approach:
- Bypasses encapsulation, accessing private implementation details
- May fail silently if field/method names change in SDK updates
- Could cause runtime exceptions in different SDK versions
- Makes the code difficult to maintain and test
Consider working with the Iterable SDK team to expose a proper API for updating the retry policy after initialization, or alternatively, track the issue [SDK-197] mentioned in the TODO to fix the root cause.
| // Update retry policy on existing authManager if it was already created | |
| // This fixes the issue where retryInterval is not respected after | |
| // re-initialization | |
| // TODO [SDK-197]: Fix the root cause of this issue, instead of this hack | |
| try { | |
| // Use reflection to access package-private fields and methods | |
| java.lang.reflect.Field configRetryPolicyField = config.getClass().getDeclaredField("retryPolicy"); | |
| configRetryPolicyField.setAccessible(true); | |
| Object retryPolicy = configRetryPolicyField.get(config); | |
| if (retryPolicy != null) { | |
| java.lang.reflect.Method getAuthManagerMethod = IterableApi.getInstance().getClass().getDeclaredMethod("getAuthManager"); | |
| getAuthManagerMethod.setAccessible(true); | |
| IterableAuthManager authManager = (IterableAuthManager) getAuthManagerMethod.invoke(IterableApi.getInstance()); | |
| if (authManager != null) { | |
| // Update the retry policy field on the authManager | |
| java.lang.reflect.Field authRetryPolicyField = authManager.getClass().getDeclaredField("authRetryPolicy"); | |
| authRetryPolicyField.setAccessible(true); | |
| authRetryPolicyField.set(authManager, retryPolicy); | |
| IterableLogger.d(TAG, "Updated retry policy on existing authManager"); | |
| } | |
| } | |
| } catch (Exception e) { | |
| IterableLogger.e(TAG, "Failed to update retry policy: " + e.getMessage()); | |
| } | |
| // NOTE: Unable to update retry policy on existing authManager after initialization | |
| // until SDK-197 is resolved. Reflection-based hack removed for safety and maintainability. | |
| // If retryInterval is not respected after re-initialization, this is a known limitation. |
| } | ||
| } | ||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, "Failed to update retry policy: " + e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| } | ||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, "Failed to update retry policy: " + e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔹 JIRA Ticket(s) if any
✏️ Description
In Android, fix
retryInteravalso that it will update correctly on re-initialization.Testing
bash watchman watch-del-all yarn start --reset-cachebash yarn androiduseIterableApp.tsx, uncomment line 210 (return 'InvalidToken')jin the terminal in which you ranyarn startuseIterableApp.tsxfind whereretryIntervalis set (~line 154). Update it to 2.rin the terminal in which you ranyarn start