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

Use more lenient timeouts when downloading package lists #1170

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

anttimaki
Copy link
Collaborator

No description provided.

Previously the package list was loaded for the default game while the
user was in the game selection screen. Now that so many games are
supported, the odds are that downloading RoR2 package list is a waste
of bandwidth.

Since the splash screen actively downloads the package list for the
selected game, triggering the background download would only slow
down both.
The package list for Lethal Company is nearing 10MB gzipped. Users with
slower connections start failing to download it within the 30s timeout,
which in practice means they can't use the mod manager.

To remedy the situation, following changes are done to timeouts:

- There's initial timeout of 30 seconds. We'd like this to be just for
  checking that the connection can be formed, there seems to be no way
  to that, so instead we check that a small amount of data has been
  transferred
- After the initial timeout, there's a timeout of 60 seconds that
  checks that data is still transferred. This timeout gets reset each
  time a download progress event fires
- Finally there's a total timeout of five minutes, which acts as a
  sanity check to prevent requests hanging forever

The solution still has many limitations:

- The download always begins from the start, so e.g. if the server
  closes the connection, reattempting the download is likely to fail
  again
- There's no check to prevent multiple package list downloads running
  simultaneously. Since the background update runs every 5 minutes, and
  the timeout is also 5 minutes, they shouldn't overlap too badly.
  However, if a user moves from a manager screen to the game selection
  screen when the background download is in progress, and then selects
  a game and gets transferred to the splash screen, it will start a
  second download which is more likely to fail because the bandwidth is
  now shared between two requests
- The UI on splash screen doesn't show any progress indicator. It seems
  the download percentage can't be shown since the total size of the
  file is unknown. I'm not sure if this is due just to the response
  headers or because the data is gzipped so the length of the actual
  content is unknown to the browser
- User has no option to manually cancel the slow download on splash
  screen. Since there's three attempts to download the package list,
  this means they might be stuck on the splash screen for 15 minutes
- From users perspective there's no difference between being offline
  (=no connection to the server) or having a slow connection, the UI
  always says they're offline if the download fails

Despite the shortcomings this should be an improvement, which gives us
more time to solve the shortcomings.
@anttimaki anttimaki changed the base branch from master to develop January 18, 2024 12:07
Comment on lines +87 to +90
// TODO: Progress percentage can't be calculated since the
// content length is unknown. Looks like this hasn't worked
// in a while.
downloadProgressed(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cloudflare seems to return the Content-Length header only if the response is resolved to an encoding format which is served by the origin. In this case, the encoding format the origin serves is gzip but cloudflare likely serves something like brotli, in which case it doesn't know the file size ahead of time. Easiest fix is to add Accept-Encoding: gzip to the request headers

@MythicManiac MythicManiac merged commit 119fb96 into develop Jan 18, 2024
6 checks passed
@MythicManiac MythicManiac deleted the package-list-timeouts branch January 18, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants