obsidian-git icon indicating copy to clipboard operation
obsidian-git copied to clipboard

[Enhancement]: Graceful offline support #590

Open bardbess opened this issue 2 years ago • 18 comments

When disconnected from the internet and Obsidian git still attempts to pull changes it throws some panicky error messages as mentioned in https://github.com/denolehov/obsidian-git/issues/590.

Check the connectivity to githubstatus before attempting to pull/push changes.

bardbess avatar Sep 05 '23 16:09 bardbess

There is a another way to implement offline-support without the dependency on github. (Not every git repo is hosted on github)

Look up the navigator.onLine function given by typescript in the docs or used by another obsidian plugin here

i think that approach is more direct and better without the dependence on a 3rd party website

rcv4 avatar Sep 06 '23 19:09 rcv4

Thanks @rcv4 I didn't know about that, would have made things much easier.

Updated the PR to use navigator.onLine.

It will now also do a pull when the online state changes and had previously been set to offlineMode.

bardbess avatar Sep 07 '23 16:09 bardbess

A little more detail on this PR if the moderators find it helpful.

From my understanding offlineMode is an existing flag that is set when repeated network attempts fail.

hasConnectivity will checks to see the device is online and If not set offlineMode saving simpleGit having to handle connectivity/network errors.

https://github.com/denolehov/obsidian-git/blob/a91330381e83cfc2ece14186325b129d7fc9b6bf/src/gitManager/simpleGit.ts#L815

When the device status changes to online AND an *event has set git-obsidian to offlineMode then reset offlineMode allowing remote attempts to retry, and also add a task to the queue.

*An event being repeated connectivity issues through the existing simpleGit offlineMode toggle or subsequent remote attempts that touch hasConnectivity.

bardbess avatar Oct 03 '23 15:10 bardbess

Sorry for taking so long to react to this pr. Where do you see these error messages? As notices in the obsidian ui or only as logs in the console? Using navigator.onLine doesn't really solve any issue, because even if it's true there still may be no internet connection.

Vinzent03 avatar Oct 08 '23 10:10 Vinzent03

Where do you see these error messages? The error messages that are annoying appear in the UI

You dont necessarily need internet to connect to, for example, a locally hosted git server.

rcv4 avatar Oct 08 '23 11:10 rcv4

Sorry for taking so long to react to this pr. Where do you see these error messages? As notices in the obsidian ui or only as logs in the console? Using navigator.onLine doesn't really solve any issue, because even if it's true there still may be no internet connection.

Easiest way to reproduce it would be to set obsidian to pull on startup, turn your device wifi/data off and start the application. The message appears in the UI as a notification on both desktop and mobile. If it tries to push new changes every X period it gets annoying pretty quick.

image

The original offlineMode error handling remains unaffected and perhaps that will suffice for when there is a network connection but the server can't be reached.

bardbess avatar Oct 08 '23 11:10 bardbess

I think the correct way to solve this is to implement the same offline detection of SimpleGit in IsomorphicGit by reading the error message. So no need for Navigator.onLine or sending additional http requests.

Vinzent03 avatar Oct 08 '23 13:10 Vinzent03

I think the correct way to solve this is to implement the same offline detection of SimpleGit in IsomorphicGit by reading the error message. So no need for Navigator.onLine or sending additional http requests.

navigator.onLine indicates wither on not your are able to connect to a local area network (LAN) or a router. If it returns false you don't have any network connectivity, local or otherwise.

SimpleGit currently attempts to send requests and after failed attempts sets the offlineMode flag to stop sending requests. I don't understand why you would send any requests if navigator.onLine returns false and your device has no network connectivity at all.

This change does not prevent catching and bubbling errors in IsomorphicGit and would complement the change you are suggestion not be superseded by it.

Here is a screenshot of Obsidian desktop with the same error (changing IsomorphicGit wont fix this).

obsidian

I have been using this build locally since raising the PR without any issues. The message after this PR on app boot. obsidian-pr

bardbess avatar Oct 08 '23 16:10 bardbess

This change does not prevent catching and bubbling errors in IsomorphicGit and would complement the change you are suggestion not be superseded by it.

I don't know what you mean with this. Could you explain it further?

I still wouldn't depend on onLine, because it might prevent requests that don't exceed, but the same issue can still occur if onLine is true. Instead, the offline error detection has to be added to IsomorphicGit and the one in SimpleGit improved as it didn't catch your error message. Sending requests that fail, but are properly caught, don't hurt in any way.

Vinzent03 avatar Oct 09 '23 08:10 Vinzent03

Hi @Vinzent03

This PR observes the network connection changing. This compliments the error handling of SimpleGit. eg:

  1. The device is connected to "network A" (perhaps a local only network), Navigator.onLine is therefore true
  2. A request is made to the github repo. This request fails, and SimpleGit sets offlineMode to true.
  3. The user switches to a new network ("network B") but offlineMode is never reset.

This PR recognizes the switch in the network, resetting offlineMode and instantly attempts a new pull request.

Another example might be travelling (train/bus/subway/etc) in areas with poor or intermittent network reception or when switching your device from flight mode.

navigator.onLine detects the network connectivity of the device, local or otherwise. So it would only prevent the same device pulling/pushing from itself when that device had no connectivity. Do you have an example where navigator.onLine might be problematic?

bardbess avatar Oct 09 '23 13:10 bardbess

navigator.onLine detects the network connectivity of the device, local or otherwise. So it would only prevent the same device pulling/pushing from itself when that device had no connectivity. Do you have an example where navigator.onLine might be problematic?

I don't know the exact behavior of navigator.onLine, but according to your definition, it'll definitively cause confusion, when the git-server is hosted in the local network or any other place which is still accessible despite github or obsidian.md not being accessible. (Which is the case in my setup.)

I like the general idea behind this feature, but would like to see one of the following alternatives:

  • only offer offline detection for specific public git hosts such as GitLab, GitHub, BitBucket, etc. If the git-url doesn't contain these, then don't change the behavior. This howeer is a bit unfair towards the people having their data not on these servers.
  • use git fetch with --dry to test connectivity. This is the best test ultimately and delegates properly to git. The response will also enable to distinguish between unreachability, connection refusal, invalid credencital/public-key. I prefer this, as it seems to be the cleanest option. However, one would need to decide, how often to test this connectivity and allow it to be configureable as well. What do the others think?

GollyTicker avatar Oct 16 '23 20:10 GollyTicker

Hi @GollyTicker

I'm afraid you misunderstood me. By saying "navigator.onLine detects the network connectivity of the device, local or otherwise", I imply that if your device is connected to any network (regardless of internet access) then navigator.onLine will return true. If however you've device is not connected to any network (airplane mode, no signal, wifi-off) it returns false.

The example I gave above - which seems absurd to me - is if you have a single PC with no network devices/configuration to anything on which you have both your git server and in some other directory an obsidian vault with a cloned git repo - as in the same device has 2 copies of your git repo - and you are pushing and pulling from the 2nd one - not sure how you could even achieve this with no network configured - an obscure and potentially impossible setup which the existing offlineMode would probably block already.

From the docs

if the browser is not able to connect to a local area network (LAN) or a router, it is offline; all other conditions return true.

bardbess avatar Oct 17 '23 00:10 bardbess

There is a another way to implement offline-support without the dependency on github. (Not every git repo is hosted on github)

Look up the navigator.onLine function given by typescript in the docs or used by another obsidian plugin here

i think that approach is more direct and better without the dependence on a 3rd party website

@rcv4 ,why is life so hard. 😮‍💨

bardbess avatar Oct 17 '23 00:10 bardbess

@bardbess

Thanks for clarifying the behaviour of navigator.online. However, what about the dry git fetch as an alternative? After all, if could be used to also give more specific feedback to the user - as many git-newcomers don't know what to do, when they for instance see an denied public key.

GollyTicker avatar Oct 17 '23 00:10 GollyTicker

Hi @GollyTicker

SimpleGit already has networkFailure error handling for this purpose, and as @Vinzent03 mentioned it would be good to implement something similar in IsomorphicGit, but why would you even attempt to fetch if your device is on airplane mode? And without leveraging the network detection handlers (used by navigator.onLine and online events) how can you trigger a pull/push once you turn airplane mode off.

bardbess avatar Oct 17 '23 13:10 bardbess

Ok, I see. Using dry fetches is a complement rather than a replacement.

I don't know too many details on the rest, so I can't comment on that.

GollyTicker avatar Oct 17 '23 14:10 GollyTicker

@GollyTicker you got it!

This PR has 2 commits:

  • The first commit just uses navigator.onLine to stop a request when there is no network connection https://github.com/denolehov/obsidian-git/pull/591/commits/e3a0be7d6a71f22c1d331c864ccbc7d068d90ade
  • The second fires an event to queue an update when the network connects https://github.com/denolehov/obsidian-git/pull/591/commits/ce07cf423580aedc4f96f00fb481fbc4bad7a492

Not sure if it helps when considering each change in isolation.

bardbess avatar Oct 17 '23 14:10 bardbess