-
Notifications
You must be signed in to change notification settings - Fork 4
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
[INV-3715] Use Network to determine online status v1 #3722
Conversation
5031fbc
to
4b4910a
Compare
4b4910a
to
0c93e6c
Compare
} | ||
yield delay(SECONDS_BETWEEN_CHECKS * 1000); | ||
} | ||
} |
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.
a single do-while loop should suffice? You have about 5 attempts going on here with a delay of 5 seconds. You can increase the MAX_ATTEMPTS and remove the outer while loop or increase SECONDS_BETWEEN_ATTEMPTS.
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.
@meghna0593
the while(true)
loop is always pinging the API to ensure continued connection unless otherwise cancelled (by a user choosing to go offline, or being disconnected and unable to reconnect)
If that do-while fetch request fails and we are UNABLE to reach our API, we don't want to immediately panic, so we send a few more requests at a faster interval to see if anything will come back. If we still can't reach the API we will trigger Offline mode.
While in offline mode, we will sporadically try a few more attempts to come back online.
In the end this is just v1 functionality, it will be succeeded by WebSockets or some similar network equivalent
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.
Originally this PR was VERY different than it is now.
The previous code you reviewed still had a handle_CHECK_MOBILE_NETWORK_STATUS
function, but the logic for it is black and white compared to the logic I explained it in call :)
Sorry for any confusion !
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.
if this was intended to be on an infinite loop when canConnect
is true
, then do you think this could be blocking in nature? Maybe making this a background task is worth looking at? @plasticviking thoughts?
Quality Gate failedFailed conditions |
Overview
This PR includes the following proposed change(s):
Handling These Network Scenarios
V2 Iteration can use Websockets to abstract all this work.
Changes: