From 48eb13d56487cc181da1dd7b2be216d7806874fa Mon Sep 17 00:00:00 2001
From: flakey5 <73616808+flakey5@users.noreply.github.com>
Date: Sun, 8 Oct 2023 10:18:06 -0700
Subject: [PATCH] code review

---
 src/constants/limits.ts                     |  5 ++++
 src/handlers/strategies/directoryListing.ts | 26 ++++++++-------------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/constants/limits.ts b/src/constants/limits.ts
index e1c0219..501e596 100644
--- a/src/constants/limits.ts
+++ b/src/constants/limits.ts
@@ -2,3 +2,8 @@
  * Max amount of retries for S3 requests
  */
 export const S3_RETRY_LIMIT = 3;
+
+/**
+ * Max amount of keys to be returned in a S3 request
+ */
+export const S3_MAX_KEYS = 1000;
diff --git a/src/handlers/strategies/directoryListing.ts b/src/handlers/strategies/directoryListing.ts
index a02bb3e..8058a41 100644
--- a/src/handlers/strategies/directoryListing.ts
+++ b/src/handlers/strategies/directoryListing.ts
@@ -12,7 +12,7 @@ import { getFile } from './serveFile';
 
 // Imports the Precompiled Handlebars Template
 import htmlTemplate from '../../templates/directoryListing.out.js';
-import { S3_RETRY_LIMIT } from '../../constants/limits';
+import { S3_MAX_KEYS, S3_RETRY_LIMIT } from '../../constants/limits';
 
 // Applies the Template into a Handlebars Template Function
 const handleBarsTemplate = Handlebars.template(htmlTemplate);
@@ -119,24 +119,22 @@ async function fetchR2Result(
   cursor: string | undefined,
   env: Env
 ): Promise<ListObjectsV2CommandOutput> {
-  let result: ListObjectsV2CommandOutput | undefined = undefined;
-
   let retriesRemaining = S3_RETRY_LIMIT;
   while (retriesRemaining > 0) {
     try {
       // Send request to R2
-      result = await client.send(
+      const result = await client.send(
         new ListObjectsV2Command({
           Bucket: env.BUCKET_NAME,
           Prefix: bucketPath,
           Delimiter: '/',
-          MaxKeys: 1000,
+          MaxKeys: S3_MAX_KEYS,
           ContinuationToken: cursor,
         })
       );
 
       // Request succeeded, no need for any retries
-      break;
+      return result;
     } catch (err) {
       // Got an error, let's log it and retry
       console.error(`R2 ListObjectsV2 error: ${err}`);
@@ -145,12 +143,8 @@ async function fetchR2Result(
     }
   }
 
-  if (result === undefined) {
-    // R2 isn't having a good day, return a 500
-    throw new Error(`R2 failed listing path ${bucketPath}`);
-  }
-
-  return result;
+  // R2 isn't having a good day, return a 500
+  throw new Error(`R2 failed listing path ${bucketPath}`);
 }
 
 /**
@@ -202,11 +196,11 @@ export async function listDirectory(
         delimitedPrefixes.add(path.Prefix.substring(bucketPath.length));
     });
 
-    const hasIndexFile = result.Contents?.find(
-      object => object.Key?.endsWith('index.html')
-    );
+    const hasIndexFile = result.Contents
+      ? result.Contents.some(object => object.Key?.endsWith('index.html'))
+      : false;
 
-    if (hasIndexFile !== undefined && hasIndexFile !== null) {
+    if (hasIndexFile) {
       return getFile(url, request, `${bucketPath}index.html`, env);
     }