From 5805a23ab49ba04e80b6e5e276fa69787bd8d5f2 Mon Sep 17 00:00:00 2001
From: flakey5 <73616808+flakey5@users.noreply.github.com>
Date: Sun, 25 Oct 2026 11:17:02 -0700
Subject: [PATCH] src: handle r2 errors in R2Middleware

Following #1067 we should be explicitly returning a 500 when we exhaust
R2 retries now

Fixes #1069

Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
---
 src/middleware/r2Middleware.ts | 71 +++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 19 deletions(-)

diff --git a/src/middleware/r2Middleware.ts b/src/middleware/r2Middleware.ts
index 5665e1e..27e3c78 100644
--- a/src/middleware/r2Middleware.ts
+++ b/src/middleware/r2Middleware.ts
@@ -11,8 +11,11 @@ import type { Middleware } from './middleware';
 import latestVersions from '../constants/latestVersions.json' assert { type: 'json' };
 import type { Request } from '../routes/request';
 import { renderDirectoryListing } from '../utils/directoryListing';
+import { env } from 'cloudflare:workers';
 import { getOriginalUrl, parseConditionalHeaders } from '../utils/request';
 import { once } from '../utils/memo';
+import internalServerError from '../responses/internalServerError';
+import type { Env } from '../env';
 
 const getProvider = once((ctx: Context) => new R2Provider({ ctx }));
 
@@ -35,6 +38,44 @@ export class R2Middleware implements Middleware {
   }
 }
 
+/**
+ * Wrap an R2 call with common error handling
+ * @param fn
+ * @returns <T> if fn didn't throw, a {@link Reponse} otherwise
+ * @throws anything that's thrown by fn and isn't an error
+ */
+async function wrapR2Call<T>(fn: () => Promise<T>): Promise<T | Response> {
+  try {
+    const result = await fn();
+
+    return result;
+  } catch (err) {
+    if (err instanceof Error) {
+      // Error object likely from R2, let's handle it here
+
+      if (err.message.includes('10020')) {
+        // R2 error: object name not valid, url probably has some weirdness in it
+        return responses.badRequest();
+      } else if (err.message.includes('10039')) {
+        // R2 error: range not compatible, probably out of bounds
+        return new Response(undefined, {
+          status: 416,
+          headers: { 'cache-control': CACHE_HEADERS.failure },
+        });
+      }
+
+      // We need to report it ourselves since we're not throwing it to the
+      // router
+      Sentry.captureException(err);
+
+      return internalServerError(err, env as Env);
+    }
+
+    // Not an error, throw just for visibility
+    throw err;
+  }
+}
+
 async function handleDirectory(
   request: Request,
   r2Path: string,
@@ -110,7 +151,11 @@ async function headFile(
   r2Path: string,
   ctx: Context
 ): Promise<Response> {
-  const result = await getProvider(ctx).headFile(r2Path);
+  const result = await wrapR2Call(() => getProvider(ctx).headFile(r2Path));
+
+  if (result instanceof Response) {
+    return result;
+  }
 
   if (result === undefined) {
     return responses.fileNotFound(request.method);
@@ -129,26 +174,14 @@ async function getFile(
 ): Promise<Response> {
   const provider = getProvider(ctx);
 
-  let result: GetFileResult | undefined;
-  try {
-    result = await provider.getFile(r2Path, {
+  const result = await wrapR2Call(() =>
+    provider.getFile(r2Path, {
       conditionalHeaders: parseConditionalHeaders(request.headers),
-    });
-  } catch (err) {
-    if (err instanceof Error) {
-      if (err.message.includes('10020')) {
-        // Object name not valid, url probably has some weirdness in it
-        return responses.badRequest();
-      } else if (err.message.includes('10039')) {
-        // Range not compatible, probably out of bounds
-        return new Response(undefined, {
-          status: 416,
-          headers: { 'cache-control': CACHE_HEADERS.failure },
-        });
-      }
-    }
+    })
+  );
 
-    throw err;
+  if (result instanceof Response) {
+    return result;
   }
 
   if (result === undefined) {