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

[Refactor] Offline detection #1727

Merged
merged 7 commits into from Oct 5, 2022
Merged

[Refactor] Offline detection #1727

merged 7 commits into from Oct 5, 2022

Conversation

arielj
Copy link
Collaborator

@arielj arielj commented Aug 14, 2022

This PR adds the offline/online detection + ping + event handling

Done:

  • check connectivity at startup
  • use the window offline and online events to detect when a network connection is active/inactive
  • show a message to the use that we are either offline or retrying the internet check
  • ping some websites we use, using axios with HEAD method to check internet connectivity
  • retry after 5 seconds if all pings fail
  • replaced the calls to isOnline with the new isOnline from online_monitor.ts
  • backend emits an event that can be listened to to do something when connection is back (check electron/anticheat/utils.ts:11)
  • added the Connection prefix for the logger
  • react app has a connectivity state that we can use anywhere if needed to show connectivity-related message (or to disable actions like opening a website or logging in for example)
  • handle heroic initialization when no internet connection (and initialize properly when it goes online), we can listen to the online event once to do things at boot waiting for the connection to be available and only once
  • more info to the user (show the list of sites we are pinging during retries)
  • i18n
  • increase the time between retries after it fails

TODO after merging this:

  • detect when an external request failed and do a ping to check if the request simply failed or we are actually offline and set the correct status to start pinging

I added a checkConnectivity function exported in online_monitor.ts that could be called after doing something that relies on an external request and we detect the connection failed. I didn't add this in this PR since I don't really know all the places where this can happen. I think it's better to start adding this calls to checkConnectivity when things fail as we find them.

TODO (I'll leave these things for the future maybe, this is more like polishing):

  • add an option to dismiss the retry message?
  • add an option retry on demand?
  • show a warning if offline and a game does not support offline mode
  • disable download/update features when offline?
  • disable verify and repair when offline?
  • disable other actions that require internet when offline?

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@arielj arielj added the pr:wip WIP, don't merge. label Aug 14, 2022
electron/anticheat/utils.ts Outdated Show resolved Hide resolved
electron/online_monitor.ts Outdated Show resolved Hide resolved
electron/main.ts Outdated Show resolved Hide resolved
@Nocccer
Copy link
Collaborator

Nocccer commented Aug 15, 2022

For me the idea looks promising. Code vice also pretty good.
Love this eventlistener:

  ipcMain.once('online', async () => {
    try {
      const { data } = await axios.default.get(
        'https://raw.githubusercontent.com/Starz0r/AreWeAntiCheatYet/HEAD/games.json'
      )
      writeFileSync(heroicAnticheatDataPath, JSON.stringify(data, null, 2))
      logInfo(`AreWeAntiCheatYet data downloaded`, LogPrefix.Backend)
    } catch (error) {
      logWarning(
        `Failed download of AreWeAntiCheatYet data: ${error}`,
        LogPrefix.Backend
      )
    }
  })

@arielj
Copy link
Collaborator Author

arielj commented Aug 15, 2022

For me the idea looks promising. Code vice also pretty good. Love this eventlistener:

  ipcMain.once('online', async () => {
    try {
      const { data } = await axios.default.get(
        'https://raw.githubusercontent.com/Starz0r/AreWeAntiCheatYet/HEAD/games.json'
      )
      writeFileSync(heroicAnticheatDataPath, JSON.stringify(data, null, 2))
      logInfo(`AreWeAntiCheatYet data downloaded`, LogPrefix.Backend)
    } catch (error) {
      logWarning(
        `Failed download of AreWeAntiCheatYet data: ${error}`,
        LogPrefix.Backend
      )
    }
  })

I'm planning to add a helper method like if we are online right now, do this, else wait until we are online, because now that is not triggered the callback if the event was fired before adding the listener, but should be a simple thing to do

@arielj arielj added pr:ready-for-review Feature-complete, ready for the grind! :P and removed pr:wip WIP, don't merge. labels Sep 7, 2022
@arielj arielj changed the title Offline detection (WIP) Offline detection Sep 7, 2022
@Nocccer Nocccer changed the title Offline detection [Refactor] Offline detection Sep 16, 2022
Copy link
Collaborator

@Nocccer Nocccer left a comment

Choose a reason for hiding this comment

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

Tested and it works. I wonder if we really need the retry. It is going on forever. It is somehow really annoying to see the timer all the time. We should be silent till the user trys something what needs internet.

Is it also possible to log less informations. Lets say just a message that we retry in 25 secs. Don't log every secs count?

My log:

(14:33:37) INFO:    [Connection]:      Connectivity: check-online
(14:33:37) INFO:    [Connection]:      Pinging external endpoints
(14:33:37) INFO:    [Legendary]:       Running command: /home/niklas/Repository/HeroicGamesLauncher/public/bin/linux/legendary --version
(14:33:37) INFO:    [Connection]:      All ping requests failed:
(14:33:37) INFO:    [Connection]:      {}
(14:33:37) INFO:    [Connection]:      Retrying in: 5 seconds
(14:33:37) INFO:    [Legendary]:       Legendary location: /home/niklas/Repository/HeroicGamesLauncher/public/bin/linux/legendary
(14:33:37) INFO:    [Gog]:             GOGDL location: /home/niklas/Repository/HeroicGamesLauncher/public/bin/linux/gogdl
(14:33:37) INFO:    [Backend]:         

Heroic Version: 2.4.3 Chopper
Legendary Version:  0.20.28 Dark Energy (hotfix #2)
OS: Pop KERNEL: 5.19.0-76051900-generic ARCH: x64
CPU: AMD Ryzen 7 3700X 8-Core Processor @3.6 GOVERNOR: schedutil
RAM: Total: 15.57 GiB Available: 11.25 GiB
GRAPHICS: GPU0: NVIDIA GeForce RTX 2060 SUPER VRAM: 8192MB DRIVER: 515.65.01 
PROTOCOL: x11

(14:33:37) WARNING: [Backend]:         Failed to register protocol with OS.
(node:39819) ExtensionLoadWarning: Warnings loading extension at /home/niklas/.config/heroic/extensions/fmkadmapgofadopljbjfkapdkoienihi:
  Manifest version 2 is deprecated, and support will be removed in 2023. See https://developer.chrome.com/blog/mv2-transition/ for more details.

(Use `electron --trace-warnings ...` to show where the warning was created)
[39819:0916/143338.104051:ERROR:browser_main_loop.cc(270)] Gtk: gtk_widget_add_accelerator: assertion 'GTK_IS_ACCEL_GROUP (accel_group)' failed
[39819:0916/143338.104129:ERROR:browser_main_loop.cc(270)] Gtk: gtk_widget_add_accelerator: assertion 'GTK_IS_ACCEL_GROUP (accel_group)' failed
[39819:0916/143338.104175:ERROR:browser_main_loop.cc(270)] Gtk: gtk_widget_add_accelerator: assertion 'GTK_IS_ACCEL_GROUP (accel_group)' failed
[39819:0916/143338.522939:ERROR:CONSOLE(160)] "Electron sandbox_bundle.js script failed to run", source: node:electron/js2c/sandbox_bundle (160)
[39819:0916/143338.522969:ERROR:CONSOLE(160)] "TypeError: object null is not iterable (cannot read property Symbol(Symbol.iterator))", source: node:electron/js2c/sandbox_bundle (160)
(14:33:38) INFO:    [Connection]:      Retrying in: 4 seconds
(14:33:39) INFO:    [Connection]:      Retrying in: 3 seconds
(14:33:40) WARNING: [DXVKInstaller]:   App offline, skipping possible DXVK update.
(14:33:40) INFO:    [Connection]:      Retrying in: 2 seconds
(14:33:40) INFO:    [Backend]:         Checking for new Heroic Updates
(14:33:40) ERROR:   [Backend]:         Error when checking for Heroic updates Error: getaddrinfo ENOTFOUND api.github.com
(14:33:40) INFO:    [Frontend]:        Refreshing Library
(14:33:40) INFO:    [Legendary]:       Refreshing library...
(14:33:40) INFO:    [Legendary]:       Refreshing Epic Games...
(14:33:40) INFO:    [Legendary]:       Game list updated, got 86 games & DLCs
(14:33:40) ERROR:   [Backend]:         Failed to get epic service status with Error: getaddrinfo ENOTFOUND status.epicgames.com
(14:33:40) INFO:    [Legendary]:       Running command: /home/niklas/Repository/HeroicGamesLauncher/public/bin/linux/legendary list
(14:33:41) INFO:    [Connection]:      Retrying in: 1 seconds
(14:33:42) INFO:    [Connection]:      Retrying in: 0 seconds
(14:33:42) INFO:    [Connection]:      Pinging external endpoints
(14:33:42) INFO:    [Connection]:      All ping requests failed:
(14:33:42) INFO:    [Connection]:      {}
(14:33:42) INFO:    [Connection]:      Retrying in: 10 seconds
(14:33:43) INFO:    [Connection]:      Retrying in: 9 seconds
(14:33:44) INFO:    [Connection]:      Retrying in: 8 seconds
(14:33:45) INFO:    [Connection]:      Retrying in: 7 seconds
(14:33:46) INFO:    [Connection]:      Retrying in: 6 seconds
(14:33:47) INFO:    [Connection]:      Retrying in: 5 seconds
(14:33:48) INFO:    [Connection]:      Retrying in: 4 seconds
(14:33:49) INFO:    [Connection]:      Retrying in: 3 seconds
(14:33:50) INFO:    [Connection]:      Retrying in: 2 seconds
(14:33:51) INFO:    [Connection]:      Retrying in: 1 seconds
(14:33:52) INFO:    [Connection]:      Retrying in: 0 seconds
(14:33:52) INFO:    [Connection]:      Pinging external endpoints
(14:33:52) INFO:    [Connection]:      All ping requests failed:
(14:33:52) INFO:    [Connection]:      {}
(14:33:52) INFO:    [Connection]:      Retrying in: 15 seconds
(14:33:53) INFO:    [Connection]:      Retrying in: 14 seconds
(14:33:54) INFO:    [Connection]:      Retrying in: 13 seconds
(14:33:55) INFO:    [Connection]:      Retrying in: 12 seconds
(14:33:56) INFO:    [Connection]:      Retrying in: 11 seconds
(14:33:57) INFO:    [Connection]:      Retrying in: 10 seconds
(14:33:58) INFO:    [Connection]:      Retrying in: 9 seconds
(14:33:59) INFO:    [Connection]:      Retrying in: 8 seconds
(14:34:00) INFO:    [Connection]:      Retrying in: 7 seconds
(14:34:01) INFO:    [Connection]:      Retrying in: 6 seconds
(14:34:02) INFO:    [Connection]:      Retrying in: 5 seconds
(14:34:03) INFO:    [Connection]:      Retrying in: 4 seconds
(14:34:04) INFO:    [Connection]:      Retrying in: 3 seconds
(14:34:05) INFO:    [Connection]:      Retrying in: 2 seconds
(14:34:06) INFO:    [Connection]:      Retrying in: 1 seconds
(14:34:07) INFO:    [Connection]:      Retrying in: 0 seconds
(14:34:07) INFO:    [Connection]:      Pinging external endpoints
(14:34:07) INFO:    [Connection]:      All ping requests failed:
(14:34:07) INFO:    [Connection]:      {}
(14:34:07) INFO:    [Connection]:      Retrying in: 20 seconds
(14:34:08) INFO:    [Connection]:      Retrying in: 19 seconds
(14:34:09) INFO:    [Connection]:      Retrying in: 18 seconds
(14:34:10) INFO:    [Connection]:      Retrying in: 17 seconds
(14:34:11) INFO:    [Connection]:      Retrying in: 16 seconds
(14:34:12) INFO:    [Connection]:      Retrying in: 15 seconds
(14:34:13) INFO:    [Connection]:      Retrying in: 14 seconds
(14:34:14) INFO:    [Connection]:      Retrying in: 13 seconds
(14:34:15) INFO:    [Connection]:      Retrying in: 12 seconds
(14:34:16) INFO:    [Connection]:      Retrying in: 11 seconds
(14:34:17) INFO:    [Connection]:      Retrying in: 10 seconds
(14:34:18) INFO:    [Connection]:      Retrying in: 9 seconds
(14:34:19) INFO:    [Connection]:      Retrying in: 8 seconds
(14:34:20) INFO:    [Connection]:      Retrying in: 7 seconds
(14:34:21) INFO:    [Connection]:      Retrying in: 6 seconds
(14:34:22) INFO:    [Connection]:      Retrying in: 5 seconds
(14:34:23) INFO:    [Connection]:      Retrying in: 4 seconds
(14:34:24) INFO:    [Connection]:      Retrying in: 3 seconds
(14:34:25) INFO:    [Connection]:      Connectivity: check-online
(14:34:25) INFO:    [Connection]:      Pinging external endpoints
(14:34:25) INFO:    [Connection]:      All ping requests failed:
(14:34:25) INFO:    [Connection]:      {}
(14:34:25) INFO:    [Connection]:      Retrying in: 25 seconds
(14:34:25) INFO:    [Connection]:      Retrying in: 2 seconds
(14:34:26) INFO:    [Connection]:      Retrying in: 24 seconds
(14:34:26) INFO:    [Connection]:      Retrying in: 1 seconds
(14:34:27) INFO:    [Connection]:      Retrying in: 23 seconds
(14:34:27) INFO:    [Connection]:      Retrying in: 0 seconds
(14:34:27) INFO:    [Connection]:      Pinging external endpoints
(14:34:27) INFO:    [Connection]:      Connectivity: online
(14:34:27) INFO:    [Gog]:             Getting data about the user
(14:34:27) INFO:    [Backend]:         AreWeAntiCheatYet data downloaded
(14:34:28) INFO:    [Gog]:             Saved user data to config

Should this be maybe also merged against beta, so we can test this more in a beta release?

Overall really good stuff and nice code :).

@arielj
Copy link
Collaborator Author

arielj commented Sep 16, 2022

Tested and it works. I wonder if we really need the retry. It is going on forever. It is somehow really annoying to see the timer all the time. We should be silent till the user trys something what needs internet.

I was planning a dismiss / x button to remove the div until the next time it goes offline. I didn't want to add that just yet to not add more things to this PR though (I want a few more things after getting this merged).

Is it also possible to log less informations. Lets say just a message that we retry in 25 secs. Don't log every secs count?

sure!

Should this be maybe also merged against beta, so we can test this more in a beta release?

I'll change it to beta today, I forgot about that (I started this against main and forgot about it hahah)

@arielj arielj changed the base branch from main to beta September 17, 2022 02:41
@arielj arielj requested a review from Nocccer September 17, 2022 02:42
@arielj
Copy link
Collaborator Author

arielj commented Sep 17, 2022

@Nocccer moved this to be against beta, removed the seconds retry log, I'll leave the dismiss (and other features) for after this is merged

Copy link
Collaborator

@Nocccer Nocccer left a comment

Choose a reason for hiding this comment

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

I found a small frontend ui problem, where the showed seconds jump between random numbers and not counting down as expected. Here is a video. I start heroic without internet and after a time i connect internet again. At this moment numbers jumping

simplescreenrecorder-2022-09-21_19.43.20.mp4

Copy link
Collaborator

@BrettCleary BrettCleary left a comment

Choose a reason for hiding this comment

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

This is cool. Definitely an important feature covering the case where connection drops out or the user starts the app offline but then goes online after launch without having to restart the app. I tested on windows 11, and it is working for me. Left some comments.

src/frontend/state/GlobalState.tsx Outdated Show resolved Hide resolved
src/frontend/helpers/onlineMonitor.ts Outdated Show resolved Hide resolved
src/frontend/components/UI/OfflineMessage/index.css Outdated Show resolved Hide resolved
src/frontend/components/UI/OfflineMessage/index.tsx Outdated Show resolved Hide resolved
src/backend/online_monitor.ts Outdated Show resolved Hide resolved
@arielj
Copy link
Collaborator Author

arielj commented Sep 26, 2022

@Nocccer can you check if the issue is still present? I was not sure how to reproduce it so I just added a check to make sure any previous timeout is stopped after starting a new timeout just in case

@arielj arielj requested a review from Nocccer October 1, 2022 23:23
@Nocccer
Copy link
Collaborator

Nocccer commented Oct 3, 2022

I still have the number bug. Here is a log. I just added a console.log(seconds) in the retry function.

(18:04:18) INFO:    [Legendary]:        Running command: /home/niklas/Repository/HeroicGamesLauncher/public/bin/linux/legendary list
1
0
(18:04:19) INFO:    [Connection]:       Pinging external endpoints
(18:04:19) INFO:    [Connection]:       All ping requests failed:
(18:04:19) INFO:    [Connection]:       [AggregateError: All promises were rejected]
10
9
8
7
6
5
4
3
2
1
0
(18:04:29) INFO:    [Connection]:       Pinging external endpoints
(18:04:29) INFO:    [Connection]:       All ping requests failed:
(18:04:29) INFO:    [Connection]:       [AggregateError: All promises were rejected]
15
14
13
12
11
(18:04:33) INFO:    [Connection]:       Connectivity: check-online
(18:04:33) INFO:    [Connection]:       Pinging external endpoints
(18:04:33) INFO:    [Connection]:       All ping requests failed:
(18:04:33) INFO:    [Connection]:       [AggregateError: All promises were rejected]
20
19
18
17
16
15
14
13
12
11
10
9
8
7
6
5
4
3
2
1
0
(18:04:53) INFO:    [Connection]:       Pinging external endpoints
(18:04:53) INFO:    [Connection]:       Connectivity: online
(18:04:53) INFO:    [Gog]:              Getting data about the user
(18:04:54) INFO:    [Backend]:          AreWeAntiCheatYet data downloaded
(18:04:54) INFO:    [Gog]:              Saved user data to config

You can see that at seconds 11 i got the connection back and the retry seconds get up to 20.

Also a big problem is that if i start heroic without internet, it always start with retry. Should it not be just showing offline and should retry if the connection is back again?

@arielj
Copy link
Collaborator Author

arielj commented Oct 3, 2022

@Nocccer thanks, I think I fixed the counter issue now, I was able to reproduce it and now it should revert back to 5 seconds after an online or offline event properly.

I also changed it to not start retrying if it's currently offline.

Copy link
Collaborator

@Nocccer Nocccer left a comment

Choose a reason for hiding this comment

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

Works like a charm now :). Really good stuff

@arielj arielj merged commit 79f40b7 into beta Oct 5, 2022
@arielj arielj deleted the feature/offline-detection branch October 5, 2022 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants