Skip to content
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

feat: handle onLine events better #246

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jan200101
Copy link
Contributor

  • adds an event handler for online and offline so we can dynamically respond if the users internet changes.
  • removed set_buttons(true) from the no-internet event because it would give back control to the user while things are still being worked on

@0neGal
Copy link
Owner

0neGal commented Aug 4, 2024

Keep in mind the online and offline not to mention navigator.onLine don't actually represent the availability of internet... Assuming it was the intention to solve #244, known good domains should be pinged to actually get the correct status.

@Jan200101
Copy link
Contributor Author

I'm well aware, this is just some boilerplate to deal with ahead of time before I get something proper out.

I'm not sure if pinging a good domain is the best course of action, since being able to connect to e.g. northstar.tf doesn't tell us if we can connect to the thunderstore since it could be down or we could be behind a misconfigured/strict firewall.

IMO the best course of action would be to deal with connection issues where they occurt (e.g. how its done in launcher.check_servers) and using navigator.onLine as a general indicator if we should even allow the user to even access the Thunderstore.

@0neGal
Copy link
Owner

0neGal commented Aug 4, 2024

I'm not sure if pinging a good domain is the best course of action, since being able to connect to e.g. northstar.tf doesn't tell us if we can connect to the thunderstore since it could be down or we could be behind a misconfigured/strict firewall.

IMO the best course of action would be to deal with connection issues where they occurt (e.g. how its done in launcher.check_servers) and using navigator.onLine as a general indicator if we should even allow the user to even access the Thunderstore.

Mostly agree here, but I do think we should check a list of known good domains i.e google.com, duckduckgo.com, northstar.tf, and if just one succeeds then we assume we're online, then handle everything else on a case by case basis...

@0neGal 0neGal changed the title handle onLine event dynamically, don't enable buttons when going offline feat: handle onLine events better Aug 4, 2024
@0neGal 0neGal added the enhancement New feature or request label Aug 4, 2024
@Jan200101
Copy link
Contributor Author

since I'm already working on extending this, should there be anything else added?
Some buttons intentionally disabled? (e.g. Find Mods), maybe a visual indicator somewhere?
Should the domain check be done regardless of if navigator.onLine is true or not?

@0neGal
Copy link
Owner

0neGal commented Aug 5, 2024

since I'm already working on extending this, should there be anything else added?

I was thinking it'd make sense to have a function like requests.check_connection(url/domain) or similar (name not important), which would simply return a bool on whether the provided URL/domain could be reached, this could then be used for both checking the online status, but also wherever else you'd want to check a specific address, i.e for the mod browser.

Some buttons intentionally disabled? (e.g. Find Mods), maybe a visual indicator somewhere?

Hmm, it might be worth adding a "no connection" type icon in the window buttons, something like this or similar, and either a tooltip informing the user of them being offline and or having it be clickable to make it show the offline toast?

I'm not aware of any other buttons that'd need disabling, which aren't already...

Should the domain check be done regardless of if navigator.onLine is true or not?

I would personally skip it, if navigator.onLine is false then it's highly unlikely there's a WAN connection.

@Jan200101
Copy link
Contributor Author

I was thinking it'd make sense to have a function like requests.check_connection(url/domain)

agreed, lets clarify how this function should behave
I don't think just pinging or checking if a host is available is a good idea since many ISPs fake that stuff.
Doing a HTTP Get is also out of the question IMO since a lot of stuff will hijack webpages (e.g. captive portals).

There are some URLs from various groups (Microsoft and KDE come to mind) that have a predefined string available at a HTTP only url to check if everything is fine and dandy
example: http://www.msftconnecttest.com/connecttest.txt

I'm not aware of any other buttons that'd need disabling, which aren't already...

Just tested with my internet disconnected, the Find Mods button is functional but clicking install on anything makes it run into an error

Error: getaddrinfo EAI_AGAIN thunderstore.io
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:108:26) {
  errno: -3001,
  code: 'EAI_AGAIN',
  syscall: 'getaddrinfo',
  hostname: 'thunderstore.io'
}

might be best to disable it when offline?

@0neGal
Copy link
Owner

0neGal commented Aug 5, 2024

agreed, lets clarify how this function should behave I don't think just pinging or checking if a host is available is a good idea since many ISPs fake that stuff. Doing a HTTP Get is also out of the question IMO since a lot of stuff will hijack webpages (e.g. captive portals).

There are some URLs from various groups (Microsoft and KDE come to mind) that have a predefined string available at a HTTP only url to check if everything is fine and dandy example: http://www.msftconnecttest.com/connecttest.txt

Perhaps add some options to it, i.e requests.check_connection(url/domain, ok_options):

ok_options = {
	// decides whether the body should be checked against anything
	body: false,

	// decides whether the response code should be anything specific
	code: false,

	// decides whether to attempt to parse the body as JSON
	is_json: false,
}

Other options could of course be added.

Just tested with my internet disconnected, the Find Mods button is functional but clicking install on anything makes it run into an error

might be best to disable it when offline?

Ah, yeah, might be a good idea, perhaps browser.toggle(false) should also be run, although I don't know if that might be a little off putting to have it simply close the popup while the user might be interacting with it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants