address PR feedback

replayio · Jul 8, 2024 · a12d367 · a12d367
1 parent ed66dd8
commit a12d367
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 17 deletions.
diff --git a/packages/shared/src/cachedFetch.ts b/packages/shared/src/cachedFetch.ts
@@ -1,4 +1,3 @@
-import assert from "node:assert/strict";
 import { fetch } from "undici";
 import { createDeferred, Deferred } from "./async/createDeferred";
 import { timeoutAfter } from "./async/timeoutAfter";
@@ -59,42 +58,47 @@ export async function cachedFetch(
       const json = await resp.json().catch(() => null);
 
       if (resp.ok) {
-        return storeCachedEntry(url, {
+        return storeCachedEntry(url, deferred, {
           json,
           ok: true,
           status: resp.status,
           statusText: resp.statusText,
         });
       }
 
-      if (shouldRetryFn) {
-        const shouldRetry = await shouldRetryFn(resp, json, retryAfter);
+      let shouldRetry = attempt < maxAttempts;
+
+      if (shouldRetry && shouldRetryFn) {
+        const shouldRetryResult = await shouldRetryFn(resp, json, retryAfter);
+
+        shouldRetry = !!shouldRetryResult;
+
         if (!shouldRetry) {
-          return storeCachedEntry(url, {
+          return storeCachedEntry(url, deferred, {
             json,
             ok: false,
             status: resp.status,
             statusText: resp.statusText,
           });
         }
-        if (typeof shouldRetry === "object" && typeof shouldRetry.after === "number") {
-          retryAfter = shouldRetry.after;
+        if (typeof shouldRetryResult === "object" && typeof shouldRetryResult.after === "number") {
+          retryAfter = shouldRetryResult.after;
         }
       }
       // If we've run out of retries, store and return the error
-      if (attempt === maxAttempts) {
-        return storeCachedEntry(url, {
+      if (!shouldRetry) {
+        return storeCachedEntry(url, deferred, {
           json,
           ok: false,
           status: resp.status,
           statusText: resp.statusText,
         });
       }
-    } catch (err) {
+    } catch (error) {
       // most likely it's a network failure, there is no need to call shouldRetryFn
       // but we should only retry if we haven't reached the maxAttempts yet
       if (attempt === maxAttempts) {
-        throw err;
+        throw error;
       }
     }
 
@@ -111,9 +115,11 @@ export function resetCache(url?: string) {
   }
 }
 
-function storeCachedEntry(url: string, entry: CacheEntry) {
-  const deferred = cache.get(url)?.deferred;
-  assert(deferred, "Deferred should be defined");
+function storeCachedEntry(
+  url: string,
+  deferred: Deferred<CacheEntry, undefined>,
+  entry: CacheEntry
+) {
   deferred.resolve(entry);
   cache.set(url, { deferred: undefined, entry });
   return entry;

diff --git a/packages/shared/src/recording/metadata/sanitizeMetadata.ts b/packages/shared/src/recording/metadata/sanitizeMetadata.ts
@@ -12,7 +12,7 @@ export async function sanitizeMetadata(metadata: UnstructuredMetadata, opts: Opt
   for (const [key, value] of Object.entries(metadata)) {
     if (typeof value !== "object") {
       if (opts.verbose) {
-        console.log(
+        logger.log(
           `Ignoring metadata key "${key}". Expected an object but received ${typeof value}`
         );
       }
@@ -26,7 +26,7 @@ export async function sanitizeMetadata(metadata: UnstructuredMetadata, opts: Opt
       switch (key) {
         case "source": {
           try {
-            const validated = await validateSource(value as UnstructuredMetadata);
+            const validated = await validateSource(value as UnstructuredMetadata | undefined);
             Object.assign(updated, validated);
           } catch (error) {
             logger.debug("Source validation failed", { error });
@@ -35,7 +35,7 @@ export async function sanitizeMetadata(metadata: UnstructuredMetadata, opts: Opt
         }
         case "test": {
           try {
-            const validated = await validateTest(value as UnstructuredMetadata);
+            const validated = await validateTest(value as UnstructuredMetadata | undefined);
             Object.assign(updated, validated);
           } catch (error) {
             logger.debug("Test validation failed", { error });