fix: prevent slow chart proxy 504s
This commit is contained in:
@@ -11,6 +11,38 @@ const DETAIL_BATCH_PROXY_TIMEOUT_MS = Number(
|
|||||||
process.env.POLYWEATHER_CITY_DETAIL_BATCH_PROXY_TIMEOUT_MS || "15000",
|
process.env.POLYWEATHER_CITY_DETAIL_BATCH_PROXY_TIMEOUT_MS || "15000",
|
||||||
);
|
);
|
||||||
|
|
||||||
|
function parseRequestedCities(req: NextRequest) {
|
||||||
|
const requestedLimit = Number(req.nextUrl.searchParams.get("limit") || "12");
|
||||||
|
const limit = Number.isFinite(requestedLimit)
|
||||||
|
? Math.max(1, Math.min(24, requestedLimit))
|
||||||
|
: 12;
|
||||||
|
const seen = new Set<string>();
|
||||||
|
const requestedCities: string[] = [];
|
||||||
|
|
||||||
|
for (const item of (req.nextUrl.searchParams.get("cities") || "").split(
|
||||||
|
",",
|
||||||
|
)) {
|
||||||
|
const city = item.trim();
|
||||||
|
if (!city || seen.has(city)) continue;
|
||||||
|
seen.add(city);
|
||||||
|
requestedCities.push(city);
|
||||||
|
if (requestedCities.length >= limit) break;
|
||||||
|
}
|
||||||
|
|
||||||
|
return requestedCities;
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildCityDetailBatchTimeoutPayload(requestedCities: string[]) {
|
||||||
|
return {
|
||||||
|
cities: requestedCities,
|
||||||
|
details: {},
|
||||||
|
errors: {},
|
||||||
|
missing: requestedCities,
|
||||||
|
partial: true,
|
||||||
|
timeout: true,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
export async function GET(req: NextRequest) {
|
export async function GET(req: NextRequest) {
|
||||||
const timer = createProxyTimer(req, "city_detail_batch");
|
const timer = createProxyTimer(req, "city_detail_batch");
|
||||||
if (!API_BASE) {
|
if (!API_BASE) {
|
||||||
@@ -25,6 +57,7 @@ export async function GET(req: NextRequest) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const forceRefresh = req.nextUrl.searchParams.get("force_refresh") ?? "false";
|
const forceRefresh = req.nextUrl.searchParams.get("force_refresh") ?? "false";
|
||||||
|
const requestedCities = parseRequestedCities(req);
|
||||||
const cachePolicy = buildCityDetailProxyCachePolicy(forceRefresh, 15);
|
const cachePolicy = buildCityDetailProxyCachePolicy(forceRefresh, 15);
|
||||||
const searchParams = new URLSearchParams({
|
const searchParams = new URLSearchParams({
|
||||||
cities: req.nextUrl.searchParams.get("cities") || "",
|
cities: req.nextUrl.searchParams.get("cities") || "",
|
||||||
@@ -52,6 +85,11 @@ export async function GET(req: NextRequest) {
|
|||||||
publicMessage: "Failed to fetch city detail batch",
|
publicMessage: "Failed to fetch city detail batch",
|
||||||
revalidateSeconds: cachePolicy.revalidateSeconds,
|
revalidateSeconds: cachePolicy.revalidateSeconds,
|
||||||
signal: controller.signal,
|
signal: controller.signal,
|
||||||
|
timeoutResponse: () =>
|
||||||
|
NextResponse.json(buildCityDetailBatchTimeoutPayload(requestedCities), {
|
||||||
|
headers: { "Cache-Control": "no-store, max-age=0" },
|
||||||
|
status: 200,
|
||||||
|
}),
|
||||||
timeoutPublicMessage: "City detail batch request timed out",
|
timeoutPublicMessage: "City detail batch request timed out",
|
||||||
timing: timer,
|
timing: timer,
|
||||||
url: `${API_BASE}/api/cities/detail-batch?${searchParams.toString()}`,
|
url: `${API_BASE}/api/cities/detail-batch?${searchParams.toString()}`,
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import {
|
|||||||
|
|
||||||
const API_BASE = process.env.POLYWEATHER_API_BASE_URL;
|
const API_BASE = process.env.POLYWEATHER_API_BASE_URL;
|
||||||
const SCAN_TERMINAL_PROXY_TIMEOUT_MS = Number(
|
const SCAN_TERMINAL_PROXY_TIMEOUT_MS = Number(
|
||||||
process.env.POLYWEATHER_SCAN_TERMINAL_PROXY_TIMEOUT_MS || "18000",
|
process.env.POLYWEATHER_SCAN_TERMINAL_PROXY_TIMEOUT_MS || "35000",
|
||||||
);
|
);
|
||||||
|
|
||||||
export const maxDuration = 45;
|
export const maxDuration = 45;
|
||||||
|
|||||||
@@ -56,6 +56,26 @@ export function runTests() {
|
|||||||
/cacheControlForData/,
|
/cacheControlForData/,
|
||||||
"city detail batch proxy should be able to suppress response caching for partial payloads",
|
"city detail batch proxy should be able to suppress response caching for partial payloads",
|
||||||
);
|
);
|
||||||
|
assert.match(
|
||||||
|
detailBatchProxy,
|
||||||
|
/buildCityDetailBatchTimeoutPayload/,
|
||||||
|
"city detail batch proxy should degrade proxy timeouts into the same partial payload shape the chart client already tolerates",
|
||||||
|
);
|
||||||
|
assert.match(
|
||||||
|
detailBatchProxy,
|
||||||
|
/missing:\s*requestedCities/,
|
||||||
|
"city detail batch proxy timeout fallback should mark requested cities as missing instead of surfacing a browser 504",
|
||||||
|
);
|
||||||
|
assert.match(
|
||||||
|
detailBatchProxy,
|
||||||
|
/partial:\s*true/,
|
||||||
|
"city detail batch proxy timeout fallback should preserve partial response semantics",
|
||||||
|
);
|
||||||
|
assert.match(
|
||||||
|
detailBatchProxy,
|
||||||
|
/status:\s*200/,
|
||||||
|
"city detail batch proxy timeout fallback should avoid red 504 fetch failures for optional chart enrichment",
|
||||||
|
);
|
||||||
assert.match(
|
assert.match(
|
||||||
apiProxySource,
|
apiProxySource,
|
||||||
/cacheControlForData\?:/,
|
/cacheControlForData\?:/,
|
||||||
@@ -65,6 +85,16 @@ export function runTests() {
|
|||||||
const scanTerminalProxy = readFrontend("app", "api", "scan", "terminal", "route.ts");
|
const scanTerminalProxy = readFrontend("app", "api", "scan", "terminal", "route.ts");
|
||||||
assert.match(scanTerminalProxy, /createProxyTimer\(req,\s*"scan_terminal"\)/);
|
assert.match(scanTerminalProxy, /createProxyTimer\(req,\s*"scan_terminal"\)/);
|
||||||
assert.match(scanTerminalProxy, /timing:\s*timer/);
|
assert.match(scanTerminalProxy, /timing:\s*timer/);
|
||||||
|
assert.match(
|
||||||
|
scanTerminalProxy,
|
||||||
|
/POLYWEATHER_SCAN_TERMINAL_PROXY_TIMEOUT_MS\s*\|\|\s*"35000"/,
|
||||||
|
"scan terminal proxy should allow the production backend enough time to return before the 45 second route cap",
|
||||||
|
);
|
||||||
|
assert.match(
|
||||||
|
scanTerminalProxy,
|
||||||
|
/export const maxDuration = 45/,
|
||||||
|
"scan terminal proxy timeout budget should remain below the Next route execution cap",
|
||||||
|
);
|
||||||
|
|
||||||
const cityDetailProxy = readFrontend("app", "api", "city", "[name]", "detail", "route.ts");
|
const cityDetailProxy = readFrontend("app", "api", "city", "[name]", "detail", "route.ts");
|
||||||
assert.match(cityDetailProxy, /createProxyTimer\(req,\s*"city_detail"\)/);
|
assert.match(cityDetailProxy, /createProxyTimer\(req,\s*"city_detail"\)/);
|
||||||
|
|||||||
@@ -124,6 +124,7 @@ export async function proxyBackendJsonGet(
|
|||||||
signal?: AbortSignal;
|
signal?: AbortSignal;
|
||||||
statusOnException?: number;
|
statusOnException?: number;
|
||||||
timeoutPublicMessage?: string;
|
timeoutPublicMessage?: string;
|
||||||
|
timeoutResponse?: () => NextResponse;
|
||||||
timing?: ProxyTimer;
|
timing?: ProxyTimer;
|
||||||
url: string;
|
url: string;
|
||||||
},
|
},
|
||||||
@@ -195,16 +196,27 @@ export async function proxyBackendJsonGet(
|
|||||||
: withCookies;
|
: withCookies;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
const timedOut = options.signal?.aborted === true;
|
const timedOut = options.signal?.aborted === true;
|
||||||
const response = buildProxyExceptionResponse(error, {
|
const response =
|
||||||
publicMessage:
|
timedOut && options.timeoutResponse
|
||||||
timedOut && options.timeoutPublicMessage
|
? options.timeoutResponse()
|
||||||
? options.timeoutPublicMessage
|
: buildProxyExceptionResponse(error, {
|
||||||
: options.publicMessage,
|
publicMessage:
|
||||||
status: timedOut ? 504 : options.statusOnException,
|
timedOut && options.timeoutPublicMessage
|
||||||
});
|
? options.timeoutPublicMessage
|
||||||
const withCookies = auth ? applyAuthResponseCookies(response, auth.response) : response;
|
: options.publicMessage,
|
||||||
|
status: timedOut ? 504 : options.statusOnException,
|
||||||
|
});
|
||||||
|
const withCookies = auth
|
||||||
|
? applyAuthResponseCookies(response, auth.response)
|
||||||
|
: response;
|
||||||
|
const outcome =
|
||||||
|
timedOut && options.timeoutResponse
|
||||||
|
? "timeout_fallback"
|
||||||
|
: timedOut
|
||||||
|
? "timeout"
|
||||||
|
: "exception";
|
||||||
return timing
|
return timing
|
||||||
? finishProxyTimedResponse(withCookies, timing, timedOut ? "timeout" : "exception")
|
? finishProxyTimedResponse(withCookies, timing, outcome)
|
||||||
: withCookies;
|
: withCookies;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -70,7 +70,7 @@ def test_scan_terminal_backend_timeout_returns_before_next_proxy_abort():
|
|||||||
ROOT / "web" / "services" / "scan_terminal_config.py"
|
ROOT / "web" / "services" / "scan_terminal_config.py"
|
||||||
).read_text(encoding="utf-8")
|
).read_text(encoding="utf-8")
|
||||||
|
|
||||||
assert 'POLYWEATHER_SCAN_TERMINAL_PROXY_TIMEOUT_MS || "18000"' in route_source
|
assert 'POLYWEATHER_SCAN_TERMINAL_PROXY_TIMEOUT_MS || "35000"' in route_source
|
||||||
assert '"POLYWEATHER_SCAN_TERMINAL_BUILD_TIMEOUT_SEC",\n 10,' in config_source
|
assert '"POLYWEATHER_SCAN_TERMINAL_BUILD_TIMEOUT_SEC",\n 10,' in config_source
|
||||||
assert (
|
assert (
|
||||||
'"POLYWEATHER_SCAN_TERMINAL_PREWARM_PAYLOAD_TIMEOUT_SEC",\n 30,'
|
'"POLYWEATHER_SCAN_TERMINAL_PREWARM_PAYLOAD_TIMEOUT_SEC",\n 30,'
|
||||||
|
|||||||
Reference in New Issue
Block a user