Extra timeout in CDN Worker's fetchText function (#5532)

Author: kamilkisiela

packages/services/cdn-worker/src/handler.ts

@@ -274,22 +274,13 @@ export function createRequestHandler(deps: RequestHandlerDependencies) {
     analytics.track({ type: 'artifact', value: artifactType, version: 'v0' }, targetId);
 
     const kvStorageKey = `target:${targetId}:${storageKeyType}`;
-    const rawValueAction = await deps
-      .getArtifactAction(targetId, null, storageKeyType, null)
-      .catch(() => {
-        // Do an extra attempt to read the value from the store.
-        // If we see that a single retry does not help, we should do a proper retry logic here.
-        // Why not now? Because we do have a new implementation that is based on R2 storage and this change is simple enough.
-        return deps.getArtifactAction(targetId, null, storageKeyType, null);
-      });
+    const rawValueAction = await deps.getArtifactAction(targetId, null, storageKeyType, null);
 
     if (rawValueAction.type === 'redirect') {
       // We're using here a private location, because the public S3 endpoint may differ from the internal S3 endpoint. E.g. within a docker network,
       // and we're fetching the artifact from within the private network.
       // If they are the same, private and public locations will be the same.
-      const rawValue = await deps
-        .fetchText(rawValueAction.location.private)
-        .catch(() => deps.fetchText(rawValueAction.location.private));
+      const rawValue = await deps.fetchText(rawValueAction.location.private);
 
       const etag = await createETag(`${kvStorageKey}|${rawValue}`);
       const ifNoneMatch = request.headers.get('if-none-match');

packages/services/cdn-worker/src/index.ts

@@ -77,13 +77,40 @@ const handler: ExportedHandler<Env> = {
       isKeyValid,
       analytics,
       async fetchText(url) {
-        const r = await fetch(url);
+        // Yeah, it's not globally defined, but it makes no sense to define it globally
+        // it's only used here and it's easier to see how it's used
+        const retries = 5;
+        const initRetryMs = 50;
 
-        if (r.ok) {
-          return r.text();
+        for (let i = 0; i <= retries; i++) {
+          const fetched = fetch(url, {
+            signal: AbortSignal.timeout(5000),
+          });
+
+          if (i === retries) {
+            const res = await fetched;
+            if (res.ok) {
+              return res.text();
+            }
+
+            throw new Error(`Failed to fetch ${url}, status: ${res.status}`);
+          }
+
+          try {
+            const res = await fetched;
+            if (res.ok) {
+              return res.text();
+            }
+          } catch (error) {
+            // Retry also when there's an exception
+            console.error(error);
+          }
+          await new Promise(resolve =>
+            setTimeout(resolve, Math.random() * initRetryMs * Math.pow(2, i)),
+          );
         }
 
-        throw new Error(`Failed to fetch ${url}, status: ${r.status}`);
+        throw new Error('An unknown error occurred, ensure retries is not negative');
       },
     });