From ae3f69946857ae75022d448bedef46c66edfc688 Mon Sep 17 00:00:00 2001
From: Noah <noahnu@gmail.com>
Date: Mon, 22 Aug 2022 09:35:03 -0400
Subject: [PATCH] feat: handle retrying on the client, not service (#328)

---
 packages/client/src/runTests.ts      | 41 +++++++++++++++++++++++++---
 packages/service/src/core/runTest.ts | 24 ++++++----------
 2 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/packages/client/src/runTests.ts b/packages/client/src/runTests.ts
index 3a3b1002..095f4458 100644
--- a/packages/client/src/runTests.ts
+++ b/packages/client/src/runTests.ts
@@ -38,11 +38,44 @@ async function runTest({
 
     const startTime = process.hrtime.bigint()
     logger.verbose(`[${invokeBackend.BackendName}] [${executionId}] Running: '${filename}'`)
-    let status: TestStatus | null = null
+    let status: TestStatus | undefined
+    let result: TestRunResult | undefined
+    let lastError: Error | undefined
     try {
-        const result = await invokeBackend.invoke({ config, filename, code, executionId })
-        status = parseStatus(result)
-        return result
+        for (let attempt = 0; attempt < config.retryCount + 1; attempt++) {
+            if (attempt > 0) {
+                // TODO: Change this to an exponential backoff?
+                await new Promise((r) => setTimeout(r, 5000 + Math.round(Math.random() * 1000)))
+            }
+
+            status = undefined
+            result = undefined
+            lastError = undefined
+
+            try {
+                result = await invokeBackend.invoke({
+                    config: {
+                        ...config,
+                        // client will take care of retrying, so we disable retrying in the service
+                        retryCount: 0,
+                    },
+                    filename,
+                    code,
+                    executionId,
+                })
+                status = parseStatus(result)
+            } catch (err) {
+                result = undefined
+                status = TestStatus.Failed
+                lastError = err instanceof Error ? err : new Error(String(err))
+                continue
+            }
+
+            if (status === TestStatus.Passed || status === TestStatus.Skipped) {
+                break
+            }
+        }
+        return result ? result : { filename, error: lastError }
     } finally {
         const duration = Number(process.hrtime.bigint() - startTime) / 1e9
         logger.verbose(
diff --git a/packages/service/src/core/runTest.ts b/packages/service/src/core/runTest.ts
index adbeaf44..7e99c9c2 100644
--- a/packages/service/src/core/runTest.ts
+++ b/packages/service/src/core/runTest.ts
@@ -29,7 +29,7 @@ export async function runTest(runTestContext: RunTestContext): Promise<InvokeRes
         'sanity_runner.max_retry_count': runTestContext.maxRetryCount,
     })
 
-    let retryCount = 0
+    const retryCount = 0
     try {
         await runner.writeTestCodeToDisk({ testCode: runTestContext.testCode })
 
@@ -46,18 +46,15 @@ export async function runTest(runTestContext: RunTestContext): Promise<InvokeRes
         }
 
         const results = await retry(
-            async () => {
-                logger.info(
-                    `Starting test run (${retryCount + 1}/${runTestContext.maxRetryCount + 1}):`,
-                    {
-                        run_id: runTestContext.runId,
-                        execution_id: runTestContext.executionId,
-                        test_file: runTestContext.testFilename,
-                    },
-                )
+            async (_, attempt) => {
+                logger.info(`Starting test run (${attempt}/${runTestContext.maxRetryCount + 1}):`, {
+                    run_id: runTestContext.runId,
+                    execution_id: runTestContext.executionId,
+                    test_file: runTestContext.testFilename,
+                })
                 const result = await trace('Test Run', async (span) => {
                     span?.addTags({
-                        'sanity_runner.attempt': retryCount + 1,
+                        'sanity_runner.attempt': attempt,
                         'sanity_runner.runner': runner.name,
                     })
                     return await runner.run()
@@ -66,7 +63,7 @@ export async function runTest(runTestContext: RunTestContext): Promise<InvokeRes
                 // force retry if test was unsuccesfull
                 // if last retry, return as normal
                 if (!result.success) {
-                    if (retryCount !== runTestContext.maxRetryCount) {
+                    if (attempt <= runTestContext.maxRetryCount) {
                         throw new Error('Test Failed!')
                     }
 
@@ -80,9 +77,6 @@ export async function runTest(runTestContext: RunTestContext): Promise<InvokeRes
             },
             {
                 retries: runTestContext.maxRetryCount,
-                onRetry: function () {
-                    retryCount++
-                },
             },
         )