[CDX-358] Handle undeliverable RxJava exceptions

[HHHindawy]

No description provided.

[constructor-claude-bedrock]

Code Review Summary

This PR addresses [CDX-358] by implementing a global RxJava error handler to gracefully handle undeliverable exceptions that can occur when network errors happen after RxJava streams have completed or been disposed. The changes include:

1. Adding a setupRxJavaErrorHandler() method to handle UndeliverableException gracefully
2. Configuring RxJava2CallAdapterFactory with an explicit IO scheduler
3. Proper exception categorization with appropriate logging

The implementation follows best practices for RxJava error handling and prevents SDK crashes from propagating to host applications. Overall, this is a solid defensive programming improvement.

---

Detailed Feedback

[File: library/src/main/java/io/constructor/core/ConstructorIo.kt Lines: 141-165] ✅ Good: Exception handling implementation

The setupRxJavaErrorHandler() method is well-structured with:

• Clear documentation explaining the purpose
• Proper unwrapping of UndeliverableException
• Appropriate categorization of network vs. unexpected exceptions
• Good logging for debugging

The approach aligns with RxJava best practices: https://github.com/ReactiveX/RxJava/wiki/What's-different-in-2.0#error-handling

[File: library/src/main/java/io/constructor/core/ConstructorIo.kt Line: 118] ⚠️ Issue: Missing error handler in testInit()

The setupRxJavaErrorHandler() is called in init() but not in testInit() (line 191). This creates inconsistent behavior between production and test initialization.

Why this matters: Tests may exhibit different error handling behavior than production, making it harder to catch issues during testing. Additionally, if tests run in parallel, they might interfere with each other's error handlers since RxJavaPlugins.setErrorHandler() sets a global singleton.

Recommendation:

internal fun testInit(context: Context?, constructorIoConfig: ConstructorIoConfig, dataManager: DataManager, preferenceHelper: PreferencesHelper, configMemoryHolder: ConfigMemoryHolder) {
    if (context == null) {
        throw IllegalStateException("Context is null, please init library using ConstructorIo.with(context)")
    }
    this.context = context.applicationContext
    setupRxJavaErrorHandler()  // Add this line
    this.dataManager = dataManager
    this.preferenceHelper = preferenceHelper
    this.configMemoryHolder = configMemoryHolder
}

[File: library/src/main/java/io/constructor/core/ConstructorIo.kt Line: 147] 💡 Suggestion: Consider idempotency

The RxJavaPlugins.setErrorHandler() sets a global error handler. If ConstructorIo.init() is called multiple times (intentionally or accidentally), the handler will be set multiple times.

Recommendation: Consider checking if a handler is already set or making the initialization idempotent:

private fun setupRxJavaErrorHandler() {
    // Only set if not already configured to avoid overwriting
    if (RxJavaPlugins.getErrorHandler() == null) {
        RxJavaPlugins.setErrorHandler { throwable ->
            // ... existing implementation
        }
    }
}

However, this might be acceptable as-is if the SDK guarantees single initialization. Please verify if multiple calls to init() are supported or guarded against elsewhere.

[File: library/src/main/java/io/constructor/core/ConstructorIo.kt Lines: 156-159] 💡 Suggestion: Consider SocketException

You're catching IOException and InterruptedException. Network operations can also throw SocketTimeoutException, SocketException, and SSLException (all subclasses of IOException), so your current code should handle them. However, for completeness and clarity, you might want to explicitly document this in the comment or add specific handling for common network exceptions if you want different logging levels.

[File: library/src/main/java/io/constructor/injection/module/NetworkModule.kt Line: 37] ✅ Good: Explicit scheduler configuration

Changing from RxJava2CallAdapterFactory.create() to RxJava2CallAdapterFactory.createWithScheduler(Schedulers.io()) is a good practice. This ensures network operations run on the IO scheduler explicitly rather than relying on default behavior.

Note: This change means all Retrofit API calls will now subscribe on Schedulers.io() by default. Ensure this doesn't conflict with any existing .subscribeOn() calls in the codebase, though typically this shouldn't be an issue as explicit subscribeOn() calls take precedence.

[General] 📝 Suggestion: Add unit tests

Consider adding unit tests to verify the error handler behavior:

1. Test that UndeliverableException with IOException is logged but doesn't throw
2. Test that UndeliverableException with unexpected exceptions is logged appropriately
3. Test that the handler unwraps nested exceptions correctly

Example test structure:

@Test
fun testSetupRxJavaErrorHandlerHandlesIOExceptionGracefully() {
    ConstructorIo.init(mockContext, testConfig)
    val handler = RxJavaPlugins.getErrorHandler()

    val ioException = IOException("Network timeout")
    val undeliverableException = UndeliverableException(ioException)

    // Should not throw
    handler.accept(undeliverableException)
}

[General] 📚 Documentation: Consider updating README

If this SDK is distributed to external developers, consider documenting this change in:

1. CHANGELOG (if one exists) - noting this as a bug fix that prevents crashes
2. README or migration guide - explaining that undeliverable exceptions are now handled gracefully

---

Conclusion

This is a well-implemented fix for a real issue with RxJava undeliverable exceptions. The code quality is good with clear comments and appropriate error categorization.

Required Changes:

• Add setupRxJavaErrorHandler() call to testInit() for consistency

Recommended Improvements:

• Consider idempotency check for the error handler
• Add unit tests for the error handling behavior
• Update documentation if applicable

Once the testInit() issue is addressed, this PR will be ready to merge. Great work on improving the SDK's stability! 🚀