Skip to content

fix: prevent 5xx spam on servers panel #3862

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
14 changes: 10 additions & 4 deletions apps/frontend/src/composables/servers/modrinth-servers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class ModrinthServer {
try {
const fileData = await useServersFetch(`/download?path=/server-icon-original.png`, {
override: auth,
retry: false,
retry: 1, // Reduce retries for optional resources
});

if (fileData instanceof Blob && import.meta.client) {
Expand All @@ -124,8 +124,14 @@ export class ModrinthServer {
return dataURL;
}
} catch (error) {
if (error instanceof ModrinthServerError && error.statusCode === 404) {
if (iconUrl) {
if (error instanceof ModrinthServerError) {
if (error.statusCode && error.statusCode >= 500) {
console.debug("Service unavailable, skipping icon processing");
sharedImage.value = undefined;
return undefined;
}

if (error.statusCode === 404 && iconUrl) {
try {
const response = await fetch(iconUrl);
if (!response.ok) throw new Error("Failed to fetch icon");
Expand Down Expand Up @@ -250,7 +256,7 @@ export class ModrinthServer {
continue;
}

if (error.statusCode === 503) {
if (error.statusCode && error.statusCode >= 500) {
console.debug(`Temporary ${module} unavailable:`, error.message);
continue;
}
Expand Down
2 changes: 2 additions & 0 deletions apps/frontend/src/composables/servers/modules/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ export class FSModule extends ServerModule {
return await requestFn();
} catch (error) {
if (error instanceof ModrinthServerError && error.statusCode === 401) {
console.debug("Auth failed, refreshing JWT and retrying");
await this.fetch(); // Refresh auth
return await requestFn();
}

throw error;
}
}
Expand Down
32 changes: 29 additions & 3 deletions apps/frontend/src/composables/servers/servers-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,23 @@ export async function useServersFetch<T>(
retry = method === "GET" ? 3 : 0,
} = options;

const circuitBreakerKey = `${module || "default"}_${path}`;
const failureCount = useState<number>(`fetch_failures_${circuitBreakerKey}`, () => 0);
const lastFailureTime = useState<number>(`last_failure_${circuitBreakerKey}`, () => 0);

const now = Date.now();
if (failureCount.value >= 3 && now - lastFailureTime.value < 30000) {
const error = new ModrinthServersFetchError(
"[Modrinth Servers] Circuit breaker open - too many recent failures",
503,
);
throw new ModrinthServerError("Service temporarily unavailable", 503, error, module);
}

if (now - lastFailureTime.value > 30000) {
failureCount.value = 0;
}

const base = (import.meta.server ? config.pyroBaseUrl : config.public.pyroBaseUrl)?.replace(
/\/$/,
"",
Expand Down Expand Up @@ -98,6 +115,7 @@ export async function useServersFetch<T>(
timeout: 10000,
});

failureCount.value = 0;
return response;
} catch (error) {
lastError = error as Error;
Expand All @@ -107,6 +125,11 @@ export async function useServersFetch<T>(
const statusCode = error.response?.status;
const statusText = error.response?.statusText || "Unknown error";

if (statusCode && statusCode >= 500) {
failureCount.value++;
lastFailureTime.value = now;
}

let v1Error: V1ErrorInfo | undefined;
if (error.data?.error && error.data?.description) {
v1Error = {
Expand Down Expand Up @@ -134,9 +157,11 @@ export async function useServersFetch<T>(
? errorMessages[statusCode]
: `HTTP Error: ${statusCode || "unknown"} ${statusText}`;

const isRetryable = statusCode ? [408, 429, 500, 502, 504].includes(statusCode) : true;
const isRetryable = statusCode ? [408, 429].includes(statusCode) : false;
const is5xxRetryable =
statusCode && statusCode >= 500 && statusCode < 600 && method === "GET" && attempts === 1;

if (!isRetryable || attempts >= maxAttempts) {
if (!(isRetryable || is5xxRetryable) || attempts >= maxAttempts) {
console.error("Fetch error:", error);

const fetchError = new ModrinthServersFetchError(
Expand All @@ -147,7 +172,8 @@ export async function useServersFetch<T>(
throw new ModrinthServerError(error.message, statusCode, fetchError, module, v1Error);
}

const delay = Math.min(1000 * Math.pow(2, attempts - 1) + Math.random() * 1000, 10000);
const baseDelay = statusCode && statusCode >= 500 ? 5000 : 1000;
const delay = Math.min(baseDelay * Math.pow(2, attempts - 1) + Math.random() * 1000, 15000);
console.warn(`Retrying request in ${delay}ms (attempt ${attempts}/${maxAttempts - 1})`);
await new Promise((resolve) => setTimeout(resolve, delay));
continue;
Expand Down