fix: surface exit code from spawned process

patch: added dotenv sample configuration and command to run locally

fix: added timeout_seconds input and handle timeout properly

.github/workflows/ci_cd.yml

@@ -42,11 +42,15 @@ jobs:
           command: npm install this-isnt-a-real-package-name-zzz
       - uses: nick-invision/assert-action@v1
         with:
-          expected: true
+          expected: 3
-          actual: ${{ steps.sad_path_wait_sec.outputs.total_attempts == '3' && steps.sad_path_wait_sec.outcome == 'failure' }}
+          actual: ${{ steps.sad_path_wait_sec.outputs.total_attempts }}
       - uses: nick-invision/assert-action@v1
         with:
-          expected: 'Command failed: npm install'
+          expected: failure
+          actual: ${{ steps.sad_path_wait_sec.outcome }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: 'Final attempt failed'
           actual: ${{ steps.sad_path_wait_sec.outputs.exit_error }}
           comparison: contains
 
@@ -60,35 +64,12 @@ jobs:
           command: node -e "process.exit(1)"
       - uses: nick-invision/assert-action@v1
         with:
-          expected: true
+          expected: 2
-          actual: ${{ steps.sad_path_error.outputs.total_attempts == '2' && steps.sad_path_error.outcome == 'failure' }}
+          actual: ${{ steps.sad_path_error.outputs.total_attempts }}
-
-      - name: sad-path (timeout)
-        id: sad_path_timeout
-        uses: ./
-        continue-on-error: true
-        with:
-          timeout_minutes: 1
-          max_attempts: 2
-          command: node -e "(async()=>await new Promise(r => setTimeout(r, 120000)))()"
       - uses: nick-invision/assert-action@v1
         with:
-          expected: true
+          expected: failure
-          actual: ${{ steps.sad_path_timeout.outputs.total_attempts == '2' && steps.sad_path_timeout.outcome == 'failure' }}
+          actual: ${{ steps.sad_path_error.outcome }}
-
-      - name: retry_on (timeout)
-        id: retry_on_timeout
-        uses: ./
-        continue-on-error: true
-        with:
-          timeout_minutes: 1
-          max_attempts: 2
-          retry_on: timeout
-          command: node -e "(async()=>await new Promise(r => setTimeout(r, 120000)))()"
-      - uses: nick-invision/assert-action@v1
-        with:
-          expected: true
-          actual: ${{ steps.retry_on_timeout.outputs.total_attempts == '2' && steps.retry_on_timeout.outcome == 'failure' }}
 
       - name: retry_on (timeout) fails early if nonzero encountered
         id: retry_on_timeout_fail
@@ -101,8 +82,16 @@ jobs:
           command: node -e "process.exit(2)"
       - uses: nick-invision/assert-action@v1
         with:
-          expected: true
+          expected: 1
-          actual: ${{ steps.retry_on_timeout_fail.outputs.total_attempts == '1' && steps.retry_on_timeout_fail.outcome == 'failure' && steps.retry_on_timeout_fail.outputs.exit_code == '2' }}
+          actual: ${{ steps.retry_on_timeout_fail.outputs.total_attempts }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: failure
+          actual: ${{ steps.retry_on_timeout_fail.outcome }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: 2
+          actual: ${{ steps.retry_on_timeout_fail.outputs.exit_code }}
 
       - name: retry_on (nonzero)
         id: retry_on_nonzero
@@ -111,26 +100,96 @@ jobs:
         with:
           timeout_minutes: 1
           max_attempts: 2
-          retry_on: timeout
+          retry_on: nonzero
           command: node -e "process.exit(2)"
       - uses: nick-invision/assert-action@v1
         with:
-          expected: true
+          expected: 2
-          actual: ${{ steps.retry_on_nonzero.outputs.total_attempts == '2' && steps.retry_on_nonzero.outcome == 'failure' && steps.retry_on_nonzero.outputs.exit_code == '2' }}
+          actual: ${{ steps.retry_on_nonzero.outputs.total_attempts }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: failure
+          actual: ${{ steps.retry_on_nonzero.outcome }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: 2
+          actual: ${{ steps.retry_on_nonzero.outputs.exit_code }}
+
+
+        # timeout tests (takes longer to run so run last)
+      - name: sad-path (timeout)
+        id: sad_path_timeout
+        uses: ./
+        continue-on-error: true
+        with:
+          timeout_seconds: 15
+          max_attempts: 2
+          command: node -e "(async()=>await new Promise(r => setTimeout(r, 120000)))()"
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: 2
+          actual: ${{ steps.sad_path_timeout.outputs.total_attempts }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: failure
+          actual: ${{ steps.sad_path_timeout.outcome }}
+
+      - name: retry_on (timeout)
+        id: retry_on_timeout
+        uses: ./
+        continue-on-error: true
+        with:
+          timeout_seconds: 15
+          max_attempts: 2
+          retry_on: timeout
+          command: node -e "(async()=>await new Promise(r => setTimeout(r, 120000)))()"
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: 2
+          actual: ${{ steps.retry_on_timeout.outputs.total_attempts }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: failure
+          actual: ${{ steps.retry_on_timeout.outcome }}
 
       - name: retry_on (nonzero) fails early if timeout encountered
         id: retry_on_nonzero_fail
         uses: ./
         continue-on-error: true
         with:
-          timeout_minutes: 1
+          timeout_seconds: 15
           max_attempts: 2
-          retry_on: timeout
+          retry_on: nonzero
           command: node -e "(async()=>await new Promise(r => setTimeout(r, 120000)))()"
       - uses: nick-invision/assert-action@v1
         with:
-          expected: true
+          expected: 1
-          actual: ${{ steps.retry_on_nonzero_fail.outputs.total_attempts == '2' && steps.retry_on_nonzero_fail.outcome == 'failure' && steps.retry_on_nonzero_fail.outputs.exit_code == '2' }}
+          actual: ${{ steps.retry_on_nonzero_fail.outputs.total_attempts }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: failure
+          actual: ${{ steps.retry_on_nonzero_fail.outcome }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: 1
+          actual: ${{ steps.retry_on_nonzero_fail.outputs.exit_code }}
+
+      - name: sad-path (timeout minutes)
+        id: sad_path_timeout_minutes
+        uses: ./
+        continue-on-error: true
+        with:
+          timeout_minutes: 1
+          max_attempts: 2
+          command: node -e "(async()=>await new Promise(r => setTimeout(r, 120000)))()"
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: 2
+          actual: ${{ steps.sad_path_timeout.outputs.total_attempts }}
+      - uses: nick-invision/assert-action@v1
+        with:
+          expected: failure
+          actual: ${{ steps.sad_path_timeout.outcome }}
 
   # runs on push to master only
   cd:

action.yml

@@ -2,8 +2,11 @@ name: Retry Step
 description: 'Retry a step on failure or timeout'
 inputs:
   timeout_minutes:
-    description: Minutes to wait before attempt times out
+    description: Minutes to wait before attempt times out.  Must only specify either minutes or seconds
-    required: true
+    required: false
+  timeout_seconds:
+    description: Seconds to wait before attempt times out.  Must only specify either minutes or seconds
+    required: false
   max_attempts:
     description: Number of attempts to make before failing the step
     required: true

dist/exec.js

@@ -1,8 +1,12 @@
-const { execSync } = require('child_process');
+const { exec } = require('child_process');
 const COMMAND = process.argv.splice(2)[0];
 
 function run() {
-  execSync(COMMAND, { stdio: 'inherit' });
+  exec(COMMAND, { stdio: 'inherit' }, (err) => {
+    if (err) {
+      process.exit(err.code);
+    }
+  });
 }
 
 run();

dist/index.js

@@ -421,7 +421,8 @@ const ms = __webpack_require__(156);
 var kill = __webpack_require__(791);
 
 // inputs
-const TIMEOUT_MINUTES = getInputNumber('timeout_minutes', true);
+const TIMEOUT_MINUTES = getInputNumber('timeout_minutes', false);
+const TIMEOUT_SECONDS = getInputNumber('timeout_seconds', false);
 const MAX_ATTEMPTS = getInputNumber('max_attempts', true);
 const COMMAND = getInput('command', { required: true });
 const RETRY_WAIT_SECONDS = getInputNumber('retry_wait_seconds', false);
@@ -439,6 +440,11 @@ function getInputNumber(id, required) {
   const input = getInput(id, { required });
   const num = Number.parseInt(input);
 
+  // empty is ok
+  if (!input && !required) {
+    return;
+  }
+
   if (!Number.isInteger(num)) {
     throw `Input ${id} only accepts numbers.  Received ${input}`;
   }
@@ -450,18 +456,51 @@ async function wait(ms) {
   return new Promise((r) => setTimeout(r, ms));
 }
 
+async function retryWait() {
+  const waitStart = Date.now();
+  await wait(ms.seconds(RETRY_WAIT_SECONDS));
+  debug(`Waited ${Date.now() - waitStart}ms`);
+  debug(`Configured wait: ${ms.seconds(RETRY_WAIT_SECONDS)}ms`);
+}
+
+async function validateInputs() {
+  if ((!TIMEOUT_MINUTES && !TIMEOUT_SECONDS) || (TIMEOUT_MINUTES && TIMEOUT_SECONDS)) {
+    throw new Error('Must specify either timeout_minutes or timeout_seconds inputs');
+  }
+
+  if (TIMEOUT_SECONDS && TIMEOUT_SECONDS < RETRY_WAIT_SECONDS) {
+    throw new Error(
+      `timeout_seconds ${TIMEOUT_SECONDS}s less than retry_wait_seconds ${RETRY_WAIT_SECONDS}s`
+    );
+  }
+}
+
+function getTimeout() {
+  if (TIMEOUT_MINUTES) {
+    return ms.minutes(TIMEOUT_MINUTES);
+  }
+
+  return ms.seconds(TIMEOUT_SECONDS);
+}
+
 async function runCmd() {
-  const end_time = Date.now() + ms.minutes(TIMEOUT_MINUTES);
+  const end_time = Date.now() + getTimeout();
 
   exit = 0;
   done = false;
 
   var child = spawn('node', [__webpack_require__.ab + "exec.js", COMMAND], { stdio: 'inherit' });
 
-  child.on('exit', (code) => {
+  child.on('exit', (code, signal) => {
+    debug(`Code: ${code}`);
+    debug(`Signal: ${signal}`);
     if (code > 0) {
       exit = code;
     }
+    // timeouts are killed manually
+    if (signal === 'SIGTERM') {
+      return;
+    }
     done = true;
   });
 
@@ -472,7 +511,7 @@ async function runCmd() {
   if (!done) {
     kill(child.pid);
     await retryWait();
-    throw new Error(`Timeout of ${TIMEOUT_MINUTES}m hit`);
+    throw new Error(`Timeout of ${getTimeout()}ms hit`);
   } else if (exit > 0) {
     await retryWait();
     throw new Error(`Child_process exited with error code ${exit}`);
@@ -481,14 +520,9 @@ async function runCmd() {
   }
 }
 
-async function retryWait() {
-  const waitStart = Date.now();
-  await wait(ms.seconds(RETRY_WAIT_SECONDS));
-  debug(`Waited ${Date.now() - waitStart}ms`);
-  debug(`Configured wait: ${ms.seconds(RETRY_WAIT_SECONDS)}ms`);
-}
-
 async function runAction() {
+  await validateInputs();
+
   for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
     try {
       // just keep overwriting attempts output
@@ -506,7 +540,7 @@ async function runAction() {
         // error: nonzero
         throw error;
       } else {
-        warning(`Attempt ${attempt} failed. Reason:`, error.message);
+        warning(`Attempt ${attempt} failed. Reason: ${error.message}`);
       }
     }
   }

package-lock.json

@@ -1501,6 +1501,12 @@
         "is-obj": "^1.0.0"
       }
     },
+    "dotenv": {
+      "version": "8.2.0",
+      "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-8.2.0.tgz",
+      "integrity": "sha512-8sJ78ElpbDJBHNeBzUbUVLsqKdccaa/BXF1uPTw3GrvQTBgrQrtObr2mUrE38vzYd8cEv+m/JBfDLioYcfXoaw==",
+      "dev": true
+    },
     "duplexer2": {
       "version": "0.1.4",
       "resolved": "https://registry.npmjs.org/duplexer2/-/duplexer2-0.1.4.tgz",

package.json

@@ -3,6 +3,7 @@
   "version": "0.0.0-managed-by-semantic-release",
   "description": "Retries a GitHub Action step on failure or timeout.",
   "scripts": {
+    "local": "node -r dotenv/config ./src/index.js",
     "prepare": "ncc build src/index.js"
   },
   "repository": {
@@ -27,6 +28,7 @@
     "@semantic-release/changelog": "^3.0.6",
     "@semantic-release/git": "^7.0.18",
     "@zeit/ncc": "^0.20.5",
+    "dotenv": "8.2.0",
     "husky": "^3.1.0",
     "semantic-release": "^17.0.3"
   },

sample.env

@@ -0,0 +1,6 @@
+INPUT_TIMEOUT_MINUTES=1
+INPUT_MAX_ATTEMPTS=3
+INPUT_COMMAND="node -e 'process.exit(99)'"
+INPUT_RETRY_WAIT_SECONDS=10
+INPUT_POLLING_INTERVAL_SECONDS=1
+INPUT_RETRY_ON=any

src/exec.js

@@ -1,8 +1,12 @@
-const { execSync } = require('child_process');
+const { exec } = require('child_process');
 const COMMAND = process.argv.splice(2)[0];
 
 function run() {
-  execSync(COMMAND, { stdio: 'inherit' });
+  exec(COMMAND, { stdio: 'inherit' }, (err) => {
+    if (err) {
+      process.exit(err.code);
+    }
+  });
 }
 
 run();

src/index.js

@@ -5,7 +5,8 @@ const ms = require('milliseconds');
 var kill = require('tree-kill');
 
 // inputs
-const TIMEOUT_MINUTES = getInputNumber('timeout_minutes', true);
+const TIMEOUT_MINUTES = getInputNumber('timeout_minutes', false);
+const TIMEOUT_SECONDS = getInputNumber('timeout_seconds', false);
 const MAX_ATTEMPTS = getInputNumber('max_attempts', true);
 const COMMAND = getInput('command', { required: true });
 const RETRY_WAIT_SECONDS = getInputNumber('retry_wait_seconds', false);
@@ -23,6 +24,11 @@ function getInputNumber(id, required) {
   const input = getInput(id, { required });
   const num = Number.parseInt(input);
 
+  // empty is ok
+  if (!input && !required) {
+    return;
+  }
+
   if (!Number.isInteger(num)) {
     throw `Input ${id} only accepts numbers.  Received ${input}`;
   }
@@ -34,18 +40,51 @@ async function wait(ms) {
   return new Promise((r) => setTimeout(r, ms));
 }
 
+async function retryWait() {
+  const waitStart = Date.now();
+  await wait(ms.seconds(RETRY_WAIT_SECONDS));
+  debug(`Waited ${Date.now() - waitStart}ms`);
+  debug(`Configured wait: ${ms.seconds(RETRY_WAIT_SECONDS)}ms`);
+}
+
+async function validateInputs() {
+  if ((!TIMEOUT_MINUTES && !TIMEOUT_SECONDS) || (TIMEOUT_MINUTES && TIMEOUT_SECONDS)) {
+    throw new Error('Must specify either timeout_minutes or timeout_seconds inputs');
+  }
+
+  if (TIMEOUT_SECONDS && TIMEOUT_SECONDS < RETRY_WAIT_SECONDS) {
+    throw new Error(
+      `timeout_seconds ${TIMEOUT_SECONDS}s less than retry_wait_seconds ${RETRY_WAIT_SECONDS}s`
+    );
+  }
+}
+
+function getTimeout() {
+  if (TIMEOUT_MINUTES) {
+    return ms.minutes(TIMEOUT_MINUTES);
+  }
+
+  return ms.seconds(TIMEOUT_SECONDS);
+}
+
 async function runCmd() {
-  const end_time = Date.now() + ms.minutes(TIMEOUT_MINUTES);
+  const end_time = Date.now() + getTimeout();
 
   exit = 0;
   done = false;
 
   var child = spawn('node', [join(__dirname, 'exec.js'), COMMAND], { stdio: 'inherit' });
 
-  child.on('exit', (code) => {
+  child.on('exit', (code, signal) => {
+    debug(`Code: ${code}`);
+    debug(`Signal: ${signal}`);
     if (code > 0) {
       exit = code;
     }
+    // timeouts are killed manually
+    if (signal === 'SIGTERM') {
+      return;
+    }
     done = true;
   });
 
@@ -56,7 +95,7 @@ async function runCmd() {
   if (!done) {
     kill(child.pid);
     await retryWait();
-    throw new Error(`Timeout of ${TIMEOUT_MINUTES}m hit`);
+    throw new Error(`Timeout of ${getTimeout()}ms hit`);
   } else if (exit > 0) {
     await retryWait();
     throw new Error(`Child_process exited with error code ${exit}`);
@@ -65,14 +104,9 @@ async function runCmd() {
   }
 }
 
-async function retryWait() {
-  const waitStart = Date.now();
-  await wait(ms.seconds(RETRY_WAIT_SECONDS));
-  debug(`Waited ${Date.now() - waitStart}ms`);
-  debug(`Configured wait: ${ms.seconds(RETRY_WAIT_SECONDS)}ms`);
-}
-
 async function runAction() {
+  await validateInputs();
+
   for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
     try {
       // just keep overwriting attempts output
@@ -90,7 +124,7 @@ async function runAction() {
         // error: nonzero
         throw error;
       } else {
-        warning(`Attempt ${attempt} failed. Reason:`, error.message);
+        warning(`Attempt ${attempt} failed. Reason: ${error.message}`);
       }
     }
   }