avoid reusing ReadableStream (#79)

vercel · Mar 17, 2023 · 2df8aac · 2df8aac
1 parent 6e85616
commit 2df8aac
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 23 deletions.
diff --git a/.changeset/seven-oranges-grin.md b/.changeset/seven-oranges-grin.md
@@ -0,0 +1,5 @@
+---
+'@vercel/edge-config': patch
+---
+
+avoid reusing ReadableStream across request contexts
diff --git a/src/index.edge.ts b/src/index.edge.ts
@@ -52,7 +52,8 @@ export function createClient(
             // the edge config itself does not exist
             throw new Error(ERRORS.EDGE_CONFIG_NOT_FOUND);
           }
-          if (res.cachedResponse) return res.cachedResponse.json();
+          if (res.cachedResponseBody !== undefined)
+            return res.cachedResponseBody as T;
           if (res.ok) return res.json();
           throw new Error(ERRORS.UNEXPECTED);
         },
@@ -109,7 +110,8 @@ export function createClient(
           // the /items endpoint never returns 404, so if we get a 404
           // it means the edge config itself did not exist
           if (res.status === 404) throw new Error(ERRORS.EDGE_CONFIG_NOT_FOUND);
-          if (res.cachedResponse) return res.cachedResponse.json();
+          if (res.cachedResponseBody !== undefined)
+            return res.cachedResponseBody as T;
           if (res.ok) return res.json();
           throw new Error(ERRORS.UNEXPECTED);
         },
@@ -123,8 +125,8 @@ export function createClient(
         headers: new Headers(headers),
       }).then(
         (res) => {
-          if (res.cachedResponse)
-            return res.cachedResponse.json() as Promise<string>;
+          if (res.cachedResponseBody !== undefined)
+            return res.cachedResponseBody as string;
           if (res.ok) return res.json() as Promise<string>;
           throw new Error(ERRORS.UNEXPECTED);
         },

diff --git a/src/index.node.ts b/src/index.node.ts
@@ -89,7 +89,8 @@ export function createClient(
             // the edge config itself does not exist
             throw new Error(ERRORS.EDGE_CONFIG_NOT_FOUND);
           }
-          if (res.cachedResponse) return res.cachedResponse.json();
+          if (res.cachedResponseBody !== undefined)
+            return res.cachedResponseBody as T;
           if (res.ok) return res.json();
           throw new Error(ERRORS.UNEXPECTED);
         },
@@ -163,7 +164,8 @@ export function createClient(
           // the /items endpoint never returns 404, so if we get a 404
           // it means the edge config itself did not exist
           if (res.status === 404) throw new Error(ERRORS.EDGE_CONFIG_NOT_FOUND);
-          if (res.cachedResponse) return res.cachedResponse.json();
+          if (res.cachedResponseBody !== undefined)
+            return res.cachedResponseBody as T;
           if (res.ok) return res.json();
           throw new Error(ERRORS.UNEXPECTED);
         },
@@ -183,8 +185,8 @@ export function createClient(
         headers: new Headers(headers),
       }).then(
         (res) => {
-          if (res.cachedResponse)
-            return res.cachedResponse.json() as Promise<string>;
+          if (res.cachedResponseBody !== undefined)
+            return res.cachedResponseBody as string;
           if (res.ok) return res.json() as Promise<string>;
           throw new Error(ERRORS.UNEXPECTED);
         },

diff --git a/src/utils/fetch-with-cached-response.test.ts b/src/utils/fetch-with-cached-response.test.ts
@@ -27,7 +27,7 @@ describe('fetchWithCachedResponse', () => {
       new Headers({ ETag: 'abc123', 'content-type': 'application/json' }),
     );
     await expect(data1.json()).resolves.toEqual({ name: 'John' });
-    expect(data1.cachedResponse).toBeUndefined();
+    expect(data1.cachedResponseBody).toBeUndefined();
 
     // Second request (should come from cache)
     fetchMock.mockResponseOnce('', {
@@ -45,10 +45,7 @@ describe('fetchWithCachedResponse', () => {
     );
 
     expect(data2).toHaveProperty('status', 304);
-    expect(data2.cachedResponse).toHaveProperty('status', 200);
-    await expect(data2.cachedResponse?.json()).resolves.toEqual({
-      name: 'John',
-    });
+    expect(data2.cachedResponseBody).toEqual({ name: 'John' });
   });
 
   it('should differentiate caches by authorization header', async () => {
@@ -93,7 +90,7 @@ describe('fetchWithCachedResponse', () => {
       new Headers({ ETag: 'abc123', 'content-type': 'application/json' }),
     );
     expect(data2).toHaveProperty('status', 200);
-    expect(data2.cachedResponse).toBeUndefined();
+    expect(data2.cachedResponseBody).toBeUndefined();
     await expect(data2.json()).resolves.toEqual({
       name: 'Bob',
     });
@@ -122,9 +119,6 @@ describe('fetchWithCachedResponse', () => {
     );
 
     expect(data3).toHaveProperty('status', 304);
-    expect(data3.cachedResponse).toHaveProperty('status', 200);
-    await expect(data3.cachedResponse?.json()).resolves.toEqual({
-      name: 'John',
-    });
+    expect(data3.cachedResponseBody).toEqual({ name: 'John' });
   });
 });
diff --git a/src/utils/fetch-with-cached-response.ts b/src/utils/fetch-with-cached-response.ts
@@ -1,12 +1,12 @@
 export interface CachedResponsePair {
   etag: string;
-  response: Response;
+  response: string;
 }
 
 type FetchOptions = Omit<RequestInit, 'headers'> & { headers?: Headers };
 
 interface ResponseWithCachedResponse extends Response {
-  cachedResponse?: Response;
+  cachedResponseBody?: unknown;
 }
 
 export const cache = new Map<string, CachedResponsePair>();
@@ -32,18 +32,23 @@ export async function fetchWithCachedResponse(
     });
 
     if (res.status === 304) {
-      res.cachedResponse = cachedResponse.clone();
+      res.cachedResponseBody = JSON.parse(cachedResponse);
       return res;
     }
 
     const newETag = res.headers.get('ETag');
     if (res.ok && newETag)
-      cache.set(cacheKey, { etag: newETag, response: res.clone() });
+      cache.set(cacheKey, {
+        etag: newETag,
+        response: await res.clone().text(),
+      });
     return res;
   }
 
   const res = await fetch(url, options);
   const etag = res.headers.get('ETag');
-  if (res.ok && etag) cache.set(cacheKey, { etag, response: res.clone() });
+  if (res.ok && etag)
+    cache.set(cacheKey, { etag, response: await res.clone().text() });
+
   return res;
 }