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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/components/mixins/UtilityMixin.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 5 additions & 8 deletions src/r2mm/connection/ConnectionProviderImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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;
Expand Down
82 changes: 79 additions & 3 deletions src/utils/HttpUtils.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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<LongRunningRequestOptions> = {}
) => {
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);
Comment on lines +87 to +90
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

}
}

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";

Expand Down
Loading