From 3865d71ef22f173b759ba529287650ac22692a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20M=C3=A4ki?= Date: Thu, 18 Jan 2024 13:03:27 +0200 Subject: [PATCH 1/2] Disable package list updates on game selection and splash screens 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. --- src/components/mixins/UtilityMixin.vue | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/components/mixins/UtilityMixin.vue b/src/components/mixins/UtilityMixin.vue index 420b48944..f40317c14 100644 --- a/src/components/mixins/UtilityMixin.vue +++ b/src/components/mixins/UtilityMixin.vue @@ -56,6 +56,15 @@ export default class UtilityMixin extends Vue { } async refreshThunderstoreModList() { + // Don't do background update on index route since the game + // isn't really chosen yet, nor in the splash screen since it + // proactively updates the package list. + const exemptRoutes = ["index", "splash"]; + + if (this.$route.name && exemptRoutes.includes(this.$route.name)) { + return; + } + const response = await ThunderstorePackages.update(GameManager.activeGame); await ApiCacheUtils.storeLastRequest(response.data); await this.$store.dispatch("updateThunderstoreModList", ThunderstorePackages.PACKAGES); From f6bfaf81d612ac79fd43b23c2e838883c318ec9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20M=C3=A4ki?= Date: Thu, 18 Jan 2024 13:27:26 +0200 Subject: [PATCH 2/2] Use more lenient timeouts when downloading package lists 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. --- src/r2mm/connection/ConnectionProviderImpl.ts | 13 ++- src/utils/HttpUtils.ts | 82 ++++++++++++++++++- 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/src/r2mm/connection/ConnectionProviderImpl.ts b/src/r2mm/connection/ConnectionProviderImpl.ts index ed4aac4b3..aa1fbc285 100644 --- a/src/r2mm/connection/ConnectionProviderImpl.ts +++ b/src/r2mm/connection/ConnectionProviderImpl.ts @@ -6,6 +6,7 @@ import GameManager from '../../model/game/GameManager'; import ConnectionProvider, { DownloadProgressed } from '../../providers/generic/connection/ConnectionProvider'; import LoggerProvider, { LogSeverity } from '../../providers/ror2/logging/LoggerProvider'; import { sleep } from '../../utils/Common'; +import { makeLongRunningGetRequest } from '../../utils/HttpUtils'; export default class ConnectionProviderImpl extends ConnectionProvider { @@ -37,14 +38,10 @@ export default class ConnectionProviderImpl extends ConnectionProvider { } private async getPackagesFromRemote(game: Game, downloadProgressed?: DownloadProgressed) { - const response = await axios.get(game.thunderstoreUrl, { - onDownloadProgress: progress => { - if (downloadProgressed !== undefined) { - downloadProgressed((progress.loaded / progress.total) * 100); - } - }, - timeout: 30000 - }); + const response = await makeLongRunningGetRequest( + game.thunderstoreUrl, + {downloadProgressed} + ) if (isApiResonse(response)) { return response as ApiResponse; diff --git a/src/utils/HttpUtils.ts b/src/utils/HttpUtils.ts index 73d177426..68c93da79 100644 --- a/src/utils/HttpUtils.ts +++ b/src/utils/HttpUtils.ts @@ -1,5 +1,7 @@ import axios from "axios"; +import { DownloadProgressed } from "../providers/generic/connection/ConnectionProvider"; + const newAbortSignal = (timeoutMs: number) => { const abortController = new AbortController(); setTimeout(() => abortController.abort(), timeoutMs); @@ -10,24 +12,98 @@ const newAbortSignal = (timeoutMs: number) => { * Return Axios instance with timeouts enabled. * @param responseTimeout Time (in ms) the server has to generate a * response once a connection is established. Defaults to 5 seconds. - * @param connectionTimeout Time (in ms) the request has in total, + * @param totalTimeout Time (in ms) the request has in total, * including opening the connection and receiving the response. * Defaults to 10 seconds. * @returns AxiosInstance */ -export const getAxiosWithTimeouts = (responseTimeout = 5000, connectionTimeout = 10000) => { +export const getAxiosWithTimeouts = (responseTimeout = 5000, totalTimeout = 10000) => { const instance = axios.create({timeout: responseTimeout}); // Use interceptors to have a fresh abort signal for each request, // so the instance can be shared by multiple requests. instance.interceptors.request.use((config) => { - config.signal = newAbortSignal(connectionTimeout); + config.signal = newAbortSignal(totalTimeout); return config; }); return instance; }; +interface LongRunningRequestOptions { + /** + * Custom function to be called when progress is made. Doesn't work + * properly currently, since the progress percentage can't be + * calculated because the total length of the content isn't known. + */ + downloadProgressed?: DownloadProgressed; + /** + * Time (in ms) the request has to trigger the first download + * progress event. This can be used to timeout early if a connection + * can't be formed at all. Defaults to 30 seconds. + */ + initialTimeout?: number; + /** + * Time (in ms) the request has in total to complete. This can be + * used as a sanity check to prevent infinite requests. Defaults to + * five minutes. + */ + totalTimeout?: number; + /** + * Time (in ms) the request has to trigger subsequent download + * progress events. This can be used to timeout the request if data + * isn't transferred fast enough or at all. Defaults to one minute. + */ + transmissionTimeout?: number; +} + +/** + * Make a GET request with extended timeouts. + * + * Since Axios's support lacks granularity, request timeouts are + * controlled with AbortController and JavaScript timeouts instead. + */ +export const makeLongRunningGetRequest = async ( + url: string, + options: Partial = {} +) => { + const { + downloadProgressed = () => null, + initialTimeout = 30 * 1000, + totalTimeout = 5 * 60 * 1000, + transmissionTimeout = 60 * 1000 + } = options; + + const abortController = new AbortController(); + const abort = () => abortController.abort(); // Set valid this. + const sanityTimeout = setTimeout(abort, totalTimeout); + let rollingTimeout = setTimeout(abort, initialTimeout); + + const onDownloadProgress = (progress: ProgressEvent) => { + clearTimeout(rollingTimeout); + rollingTimeout = setTimeout(abort, transmissionTimeout); + + if (typeof downloadProgressed === "function") { + // TODO: Progress percentage can't be calculated since the + // content length is unknown. Looks like this hasn't worked + // in a while. + downloadProgressed(0); + } + } + + const instance = axios.create({ + onDownloadProgress, + signal: abortController.signal, + }); + + try { + return await instance.get(url); + } finally { + clearTimeout(sanityTimeout); + clearTimeout(rollingTimeout); + } +} + export const isNetworkError = (responseOrError: unknown) => responseOrError instanceof Error && responseOrError.message === "Network Error";