-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: implement connectivity observer #102
base: develop
Are you sure you want to change the base?
feat: implement connectivity observer #102
Conversation
This is to ensure that we don't need to do un-necessary casting of application object
For better readability.
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.
Great effort!! 🚀
*/ | ||
@InternalRudderApi | ||
@Suppress("TooGenericExceptionCaught") | ||
inline fun safelyExecute(block: () -> Unit, onException: (Exception) -> Unit, onFinally: () -> Unit = {}) { |
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.
I am having some doubts on the safelyExecute
method. I understand that it encapsulates and forces the try
/ catch
/ finally
block, but at the same time it hides the code and does not promote much readability. I need some time to see how this would work. What were you trying to achieve with this method?
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.
For this, I'm trying to achieve two use cases:
- Encapsulate the boilerplate try-catch code.
- In the future, when error collection tools are implemented in our SDK, we can utilize the catch block to track and notify any exceptions. This will ensure that code modifications are not required everywhere; only a single change will be needed.
e.g., something like this:
inline fun safelyExecute(block: () -> Unit, onException: (Exception) -> Unit, onFinally: () -> Unit = {}) {
try {
block()
} catch (e: Exception) {
onException(e)
ExceptionTracker.notify("Some exception occurred: $e"); // We only need a single change to track and notify exceptions.
} finally {
onFinally()
}
}
Please let me know, if it doesn't help in promoting readability then I think it would be better to directly use the try-catch block.
Description
Type of change
Implementation Details
ConnectivityState
core
module to always default totrue
.true
and anyone watching on this variable will be notified immediately after calling this action.false
and anyone watching on this variable will be notified immediately after calling this action.AndroidConnectivityObserverPlugin
N = 24
or above).true
.Checklist
How to test?
Breaking Changes
Maintainers Checklist
Screenshots (if applicable)
Additional Context
I'll implement the TODO directly in the SourceConfig PR.