connectionStatus.startChecking() should start checking with a default interval of 30s

[gr2m]

follow up for https://github.com/hoodiehq/hoodie-camp-tutorial/issues/6

right now nothing happens when I run connectionStatus.startChecking(). The expected behavior would be to send a check request immediately, and then another one every 30s by default, or what ever is set in options.interval when initialising the API: https://github.com/hoodiehq/hoodie-connection-status#constructor

[gr2m]

@antoinelyset asked on Hoodie Chat

Hi Gregor, I don't really understand the handleInterval in the the file lib/start-checking.js . Is an interval the same thing as a timeout ? I'm not a good programmer in JS (yet :simple_smile: ) but I find quite confusing the variable rewriting of timeout in each case.

[gr2m]

I agree that the wording here can be confusing. We call it "interval" because the startChecking method checks if the connection is okay based on a time interval, similar to JavaScript’s setInterval method.

But we don’t use the setInterval method, because it would send a request e.g. every 3s. If you are on a bad connection the request roundtrip can sometimes take longer than that, so you end up sending another request before the response for the first one arrived. That’s why we use setTimeout here, the timer only starts after the response from the last request arrived. The request is sent in internals.check, that’s the same as if the user would use the public connectionStatus.check() API.

Does that make all sense? Maybe we can change the wording or add comments to clarify this

[antoinelyset]

Yeah sure. I get it ! I think I confused the checkTimeout option with the timeoutone. My bad ! It's clearer now ! Thanks

[gr2m]

Do you want to work on it? Feel free to assign yourself, it’s all yours, I’m here to assist if you have any farther questions

[antoinelyset]

Sure ! I will try tomorrow !

[antoinelyset]

I'm not able to find the time and the knowledge to do this. I will give up on this one. Sorry Hoodie team, but I'm uselessly blocking this issue.

[varjmes]

Hey @antoinelyset! Thanks so much for attempting this, and there is no need whatsoever to apologise :)

[minrwhite]

@gr2m As per the slack chat, I'm claiming this

[minrwhite]

The related PR will pass Travis checks, however this still in progress, due to the skipped 'startChecking() with invalid options' test, which now otherwise fails due to the default interval now allowing a blank object to pass the checks.

To truly meet the spirit of this skipped test I suggest we make startChecking() a gatekeeper that ensures that required options of check() are present before allowing an interval to be set - should this be an expanded scope of this issue or a separate issue created?

[gr2m]

Yeah I’d suggest lets fix it with this PR, it looks good so far, thanks!

[minrwhite]

Fixed and pushed, ready for review

[minrwhite]

Review changes pushed, again ready for review.