Improve Retry Mechanism and Listener Handling in ResumeTask

[arslanarso]

Improve Retry Mechanism and Listener Handling in ResumeTask

This PR introduces two key improvements to the ResumeTask implementation:

1. Move this._resumeRetryN += 1 to _scheduleResume:

• The retry counter is now incremented only when a resume task is actually scheduled.
• This ensures that the retry count accurately reflects the number of retry attempts, avoiding unnecessary increments when _scheduleResume is not invoked.

2. Remove unnecessary call to _scheduleResume in schedule:

• Previously, _scheduleResume was invoked in schedule regardless of the network status, potentially causing attempts to reconnect even when the network was offline.
• This line has been removed to ensure that reconnect attempts are driven solely by the network listener, making the logic more consistent and reliable.

---

These changes improve the accuracy and robustness of the retry mechanism and eliminate potential conflicts in listener handling.

[jitsi-jenkins]

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

[arslanarso]

Hi @saghul I have signed the CLA. kindly review my PR now

[saghul]

Both of the changes break the logic in subtle ways.

1. Move this._resumeRetryN += 1 to _scheduleResume:

* The retry counter is now incremented only when a resume task is actually scheduled.

* This ensures that the retry count accurately reflects the number of retry attempts, avoiding unnecessary increments when `_scheduleResume` is not invoked.

In order for the retry count check to work, the counter needs to be accurate when we call schedule(), see how it's used in XmppConnection.js. The point of the counter is to avoid scheduling more than X times.

2. Remove unnecessary call to _scheduleResume in schedule:

* Previously, `_scheduleResume` was invoked in `schedule` regardless of the network status, potentially causing attempts to reconnect even when the network was offline.

* This line has been removed to ensure that reconnect attempts are driven solely by the network listener, making the logic more consistent and reliable.

It's not unnecessary. There are network glitches that will cause the connection to break while the network state doesn't transition to offline, so we cannot assume schedule was called in an offline state.

[arslanarso]

2. Remove unnecessary call to _scheduleResume in schedule:

* Previously, `_scheduleResume` was invoked in `schedule` regardless of the network status, potentially causing attempts to reconnect even when the network was offline.

* This line has been removed to ensure that reconnect attempts are driven solely by the network listener, making the logic more consistent and reliable.

It's not unnecessary. There are network glitches that will cause the connection to break while the network state doesn't transition to offline, so we cannot assume schedule was called in an offline state.

I observed that with this line NetworkInfo.isOnline() && this._scheduleResume();, the _scheduleResume function is triggered even when the internet is disconnected. Normally, it should not trigger the _scheduleResume function immediately when the internet is lost.

What solution should be applied here, in your opinion? @saghul

[saghul]

I think you are looking at this the wrong way. The real question is why does NetworkInfo.isOnline() return true if you are not online?

At the end of the day, it doesn't matter much. If your internet is down, but we retry, the connection will fail, and when the retry-count is exhausted, the reload mechanism will kick in. Otherwise you are going to be left in the meeting with no audio / video and wondering what's going on.

What exactly are you trying to fix here?