Skip to content

Add 10 second timeout waiting for connection before client behaves as-if offline. #872

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

Merged
merged 1 commit into from
Mar 5, 2018

Conversation

mikelehen
Copy link
Contributor

[Port of https://github.com/firebase/firebase-js-sdk/commit/0fa319e5e019dd0d40ab441d2ff9f8f6d4724e43]

  • Refactored FSTOnlineState tracking out of FSTRemoteStore and into new
    FSTOnlineStateTracker component.
  • Added a 10 second timeout to transition from OnlineState.Unknown to
    OnlineState.Offline rather than waiting indefinitely for the stream to
    succeed or fail.
  • Removed hack to run SpecTests using an FSTDispatchQueue that wrapped
    dispatch_get_main_queue(). This was incompatible with [FSTDispatchQueue
    runDelayedCallbacksUntil:] since it queues work and blocks waiting for
    it to complete. Now spec tests create / use a proper FSTDispatchQueue.
  • Added a SpecTest to verify OnlineState timeout behavior.
  • Misc cleanup:
    • Renamed FSTOnlineState states: Failed => Offline, Healthy => Online
    • Renamed FSTTimerIds (ListenStreamConnection => ListenStreamConnectionBackoff)
    • Added ability to run timers from spec tests.

…-if offline.

[Port of firebase/firebase-js-sdk@0fa319e]

* Refactored FSTOnlineState tracking out of FSTRemoteStore and into new
  FSTOnlineStateTracker component.
* Added a 10 second timeout to transition from OnlineState.Unknown to
  OnlineState.Offline rather than waiting indefinitely for the stream to
  succeed or fail.
* Removed hack to run SpecTests using an FSTDispatchQueue that wrapped
  dispatch_get_main_queue(). This was incompatible with [FSTDispatchQueue
  runDelayedCallbacksUntil:] since it queues work and blocks waiting for
  it to complete. Now spec tests create / use a proper FSTDispatchQueue.
* Added a SpecTest to verify OnlineState timeout behavior.
* Misc cleanup:
    * Renamed FSTOnlineState states: Failed => Offline, Healthy => Online
    * Renamed FSTTimerIds (ListenStreamConnection => ListenStreamConnectionBackoff)
    * Added ability to run timers from spec tests.
Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

*
* It sets the FSTOnlineState to Unknown and starts the onlineStateTimer if necessary.
*/
- (void)handleWatchStreamStart;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obj-C style would have us name this something like watchStreamDidStart but we can also just leave this because we'll be porting it to C++ soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if that convention applied since this isn't actually a delegate method or anything. Anyway, I think I'll leave as-is and we can revisit during porting as necessary. Thanks!

@wilhuff wilhuff assigned mikelehen and unassigned wilhuff Mar 5, 2018
@mikelehen mikelehen merged commit 34ebf10 into master Mar 5, 2018
@mikelehen mikelehen deleted the mikelehen/onlinestate-timeout branch March 5, 2018 17:49
minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request Jun 6, 2018
…-if offline. (firebase#872)

[Port of firebase/firebase-js-sdk@0fa319e]

* Refactored FSTOnlineState tracking out of FSTRemoteStore and into new
  FSTOnlineStateTracker component.
* Added a 10 second timeout to transition from OnlineState.Unknown to
  OnlineState.Offline rather than waiting indefinitely for the stream to
  succeed or fail.
* Removed hack to run SpecTests using an FSTDispatchQueue that wrapped
  dispatch_get_main_queue(). This was incompatible with [FSTDispatchQueue
  runDelayedCallbacksUntil:] since it queues work and blocks waiting for
  it to complete. Now spec tests create / use a proper FSTDispatchQueue.
* Added a SpecTest to verify OnlineState timeout behavior.
* Misc cleanup:
    * Renamed FSTOnlineState states: Failed => Offline, Healthy => Online
    * Renamed FSTTimerIds (ListenStreamConnection => ListenStreamConnectionBackoff)
    * Added ability to run timers from spec tests.
@firebase firebase locked and limited conversation to collaborators Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants