Merge pull request #107 from semrush/retries-and-attempts

Retries and attempts
semrush · Dec 11, 2024 · 72affb8 · 72affb8
2 parents 2988c35 + 40571a8
commit 72affb8
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 26 deletions.
diff --git a/docker/Dockerfile b/docker/Dockerfile
@@ -1,6 +1,6 @@
 ARG NODE_VERSION=21.7
 
-FROM node:${NODE_VERSION} as vendor
+FROM node:${NODE_VERSION} AS vendor
 
 ENV PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true
 
@@ -13,12 +13,13 @@ RUN npm ci --omit dev
 
 FROM node:${NODE_VERSION}
 
-ARG CHROMIUM_VERSION=130.0.6723.116-1~deb12u1
+ARG CHROMIUM_VERSION=131.0.*
 ENV CHROMIUM_VERSION=${CHROMIUM_VERSION}
 ENV PUPPETEER_EXECUTABLE_PATH=/usr/bin/chromium
 
 RUN apt update && \
     apt install -y --no-install-recommends \
+      chromium-common=${CHROMIUM_VERSION}  \
       chromium=${CHROMIUM_VERSION} \
       chromium-sandbox=${CHROMIUM_VERSION} && \
     apt clean && \

diff --git a/src/check/__tests__/runner.test.js b/src/check/__tests__/runner.test.js
@@ -79,7 +79,8 @@ test('check-with-exception', () => {
     expect(report).toBeInstanceOf(CheckReport);
     expect(report).toEqual(
       expect.objectContaining({
-        shortMessage: "Action 'errorAction' failed: This action must fail",
+        shortMessage:
+          "Action 'errorAction' failed after 1 attempts: This action must fail",
         success: false,
       })
     );

diff --git a/src/check/runner.js b/src/check/runner.js
@@ -159,10 +159,9 @@ class CheckRunner {
     }
 
     let har;
-
     if (config.hars) {
-      checkReport.harPath = paths.getHarPath();
       har = new PuppeteerHar(page);
+      checkReport.harPath = paths.getHarPath();
       await har.start({ path: paths.getHarTempPath() });
     }
 
@@ -180,17 +179,22 @@ class CheckRunner {
       const [stepName, stepParameters] = scenarioCopy[step];
 
       result = result.then(async () => {
-        const actionPromise = CheckRunner.doAction(
-          page,
-          stepName,
-          stepParameters
-        );
-
         const actionReport = new ActionReport(stepName, step, '***hidden***');
-        actionReport.startDateTime = new Date().toISOString();
+        const maxRetries = 5;
+        const baseDelay = 1000;
+
+        // eslint-disable-next-line consistent-return
+        const runAction = async (attempt = 1) => {
+          actionReport.startDateTime = new Date().toISOString();
 
-        await actionPromise
-          .then(async (actionResult) => {
+          try {
+            const actionPromise = CheckRunner.doAction(
+              page,
+              stepName,
+              stepParameters
+            );
+
+            const actionResult = await actionPromise;
             actionReport.success = true;
 
             if (actionResult instanceof CheckReportCustomData) {
@@ -207,23 +211,42 @@ class CheckRunner {
                   });
                 })
                 .catch((err) => {
-                  log.error('Could not get a cookies: ', err);
+                  log.error('Could not get cookies: ', err);
                 });
             }
-          })
-          .catch((err) => {
+          } catch (err) {
+            const isResetError = err.message.includes('ERR_CONNECTION_RESET');
+            const isClosedError = err.message.includes('ERR_CONNECTION_CLOSED');
+            if (isResetError || isClosedError) {
+              log.error('ERR_CONNECTION_* error occurred: ', err);
+            }
+            if ((isResetError || isClosedError) && attempt <= maxRetries) {
+              const jitter = Math.random() * 1000;
+              const delay = baseDelay * attempt + jitter;
+              log.warn(
+                `Retrying action '${stepName}' (Attempt ${attempt} of ${maxRetries}) after ${delay.toFixed(
+                  0
+                )}ms due to connection issue.`
+              );
+              await new Promise((resolve) => {
+                setTimeout(resolve, delay);
+              });
+              return runAction(attempt + 1);
+            }
             actionReport.success = false;
             actionReport.shortMessage = err.message;
             actionReport.fullMessage = err;
             throw utils.enrichError(
               err,
-              `Action '${stepName}' failed: ${err.message}`
+              `Action '${stepName}' failed after ${attempt} attempts: ${err.message}`
             );
-          })
-          .finally(() => {
+          } finally {
             actionReport.endDateTime = new Date().toISOString();
             checkReport.actions.push(actionReport);
-          });
+          }
+        };
+
+        await runAction();
       });
     }
 
@@ -289,7 +312,9 @@ class CheckRunner {
 
         if (config.hars) {
           try {
-            await har.stop();
+            if (har.inProgress === true) {
+              await har.stop();
+            }
           } catch (err) {
             Sentry.captureException(err);
             log.error('Can not stop har generation: ', err);
@@ -302,17 +327,23 @@ class CheckRunner {
           !config.artifactsKeepSuccessful
         ) {
           if (config.traces) {
-            fs.unlinkSync(paths.getTraceTempPath());
+            if (fs.existsSync(paths.getTraceTempPath())) {
+              fs.unlinkSync(paths.getTraceTempPath());
+            }
           }
           if (config.hars) {
-            fs.unlinkSync(paths.getHarTempPath());
+            if (fs.existsSync(paths.getHarTempPath())) {
+              fs.unlinkSync(paths.getHarTempPath());
+            }
           }
           return;
         }
 
         if (config.traces) {
           try {
-            utils.moveFile(paths.getTraceTempPath(), paths.getTracePath());
+            if (fs.existsSync(paths.getTraceTempPath())) {
+              utils.moveFile(paths.getTraceTempPath(), paths.getTracePath());
+            }
           } catch (err) {
             Sentry.captureException(err);
             log.error('Can not save trace file: ', err);
@@ -321,7 +352,9 @@ class CheckRunner {
 
         if (config.hars) {
           try {
-            utils.moveFile(paths.getHarTempPath(), paths.getHarPath());
+            if (fs.existsSync(paths.getHarTempPath())) {
+              utils.moveFile(paths.getHarTempPath(), paths.getHarPath());
+            }
           } catch (err) {
             Sentry.captureException(err);
             log.error('Can not save HAR file: ', err);