From ef54e4e6cc991fc4b4bc53ad1f38ab9f821c751e Mon Sep 17 00:00:00 2001 From: Xi Xu Date: Tue, 22 Jul 2025 12:19:54 +0800 Subject: [PATCH] Improve Git request handling and error responses Refactors Git request processing to set the correct Content-Type and include the Git-Protocol header. Enhances retry logic by avoiding retries on client errors (4xx), adds security headers to error responses, and improves error handling for timeouts and upstream server errors. --- src/index.js | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/src/index.js b/src/index.js index 6a0edd8..1e2a933 100644 --- a/src/index.js +++ b/src/index.js @@ -1,4 +1,4 @@ -import { CONFIG } from "./config"; +import { CONFIG } from "./config/index.js"; /** * Monitors performance metrics during request processing @@ -182,7 +182,7 @@ async function handleRequest(request, env, ctx) { // Add body for POST requests (Git operations) if (request.method === "POST" && isGit) { - fetchOptions.body = request.body; + fetchOptions.body = await request.arrayBuffer(); } // Set appropriate headers for Git vs regular requests @@ -195,6 +195,7 @@ async function handleRequest(request, env, ctx) { "User-Agent", "Accept", "Accept-Encoding", + "Git-Protocol", ]; gitHeaders.forEach((header) => { @@ -208,6 +209,17 @@ async function handleRequest(request, env, ctx) { if (!fetchOptions.headers.has("User-Agent")) { fetchOptions.headers.set("User-Agent", "git/2.34.1"); } + + // Ensure proper content type for Git operations + if ( + request.method === "POST" && + !fetchOptions.headers.has("Content-Type") + ) { + fetchOptions.headers.set( + "Content-Type", + "application/x-git-upload-pack-request" + ); + } } else { // Regular file download headers Object.assign(fetchOptions, { @@ -261,6 +273,12 @@ async function handleRequest(request, env, ctx) { break; } + // Don't retry on client errors (4xx) - these won't improve with retries + if (response.status >= 400 && response.status < 500) { + monitor.mark("client_error"); + break; + } + attempts++; if (attempts < CONFIG.MAX_RETRIES) { await new Promise((resolve) => @@ -270,17 +288,47 @@ async function handleRequest(request, env, ctx) { } catch (error) { attempts++; if (error.name === "AbortError") { - return new Response("Request timeout", { status: 408 }); + return new Response("Request timeout", { + status: 408, + headers: addSecurityHeaders(new Headers()), + }); } if (attempts >= CONFIG.MAX_RETRIES) { return new Response( `Failed after ${CONFIG.MAX_RETRIES} attempts: ${error.message}`, - { status: 500 } + { + status: 500, + headers: addSecurityHeaders(new Headers()), + } ); } + // Wait before retrying + await new Promise((resolve) => + setTimeout(resolve, CONFIG.RETRY_DELAY_MS * attempts) + ); } } + // Check if we have a valid response after all attempts + if (!response) { + return new Response("No response received after all retry attempts", { + status: 500, + headers: addSecurityHeaders(new Headers()), + }); + } + + // If response is still not ok after all retries, return the error + if (!response.ok && response.status !== 206) { + const errorText = await response.text().catch(() => "Unknown error"); + return new Response( + `Upstream server error (${response.status}): ${errorText}`, + { + status: response.status, + headers: addSecurityHeaders(new Headers()), + } + ); + } + // Prepare response headers const headers = new Headers(response.headers);