♨️ Hot fix (recorder): don't mess retries order any more (#2668)

* don't change the original reties order any more

* reverse retryRules only once

* add test

* add unit tests

Co-authored-by: Mario Melcher <mario.melcher@seitenbau.com>

Author: nitschSB

lib/actor.js

@@ -35,8 +35,6 @@ class Actor {
   retry(opts) {
     if (opts === undefined) opts = 1;
     recorder.retry(opts);
-    // adding an empty promise to clear retries
-    recorder.add(() => null);
     // remove retry once the step passed
     recorder.add(() => event.dispatcher.once(event.step.finished, () => recorder.retries.pop()));
     return this;

lib/recorder.js

@@ -181,11 +181,9 @@ module.exports = {
         return Promise.resolve(res).then(fn);
       }
 
+      const retryRules = this.retries.slice().reverse();
       return promiseRetry(Object.assign(defaultRetryOptions, retryOpts), (retry, number) => {
         if (number > 1) log(`${currentQueue()}Retrying... Attempt #${number}`);
-
-        const retryRules = this.retries.reverse();
-
         return Promise.resolve(res).then(fn).catch((err) => {
           for (const retryObj of retryRules) {
             if (!retryObj.when) return retry(err);

test/unit/actor_test.js

@@ -21,15 +21,16 @@ describe('Actor', () => {
         bye: () => 'bye world',
         die: () => { throw new Error('ups'); },
         _hidden: () => 'hidden',
-        failFirst: () => {
+        failAfter: (i = 1) => {
           counter++;
-          if (counter < 2) throw new Error('ups');
+          if (counter <= i) throw new Error('ups');
+          counter = 0;
         },
       },
       MyHelper2: {
         greeting: () => 'greetings, world',
       },
-    });
+    }, undefined, undefined);
     container.translation().vocabulary.actions.hello = 'привет';
     I = actor();
     event.cleanDispatcher();
@@ -79,7 +80,7 @@ describe('Actor', () => {
   });
 
   it('should take all methods from helpers and built in', () => {
-    ['hello', 'bye', 'die', 'failFirst', 'say', 'retry', 'greeting'].forEach(key => {
+    ['hello', 'bye', 'die', 'failAfter', 'say', 'retry', 'greeting'].forEach(key => {
       expect(I).toHaveProperty(key);
     });
   });
@@ -108,11 +109,40 @@ describe('Actor', () => {
   });
 
   it('should retry failed step with #retry', () => {
-    return I.retry(2).failFirst();
+    recorder.start();
+    return I.retry({ retries: 2, minTimeout: 0 }).failAfter(1);
   });
 
   it('should retry once step with #retry', () => {
-    return I.retry().failFirst();
+    recorder.start();
+    return I.retry().failAfter(1);
+  });
+
+  it('should alway use the latest global retry options', () => {
+    recorder.start();
+    recorder.retry({
+      retries: 0,
+      minTimeout: 0,
+      when: () => true,
+    });
+    recorder.retry({
+      retries: 1,
+      minTimeout: 0,
+      when: () => true,
+    });
+    I.hello(); // before fix: this changed the order of retries
+    return I.failAfter(1);
+  });
+
+  it('should not delete a global retry option', () => {
+    recorder.start();
+    recorder.retry({
+      retries: 2,
+      minTimeout: 0,
+      when: () => true,
+    });
+    I.retry(1).failAfter(1); // before fix: this changed the order of retries
+    return I.failAfter(2);
   });
 
   it('should print handle failed steps', () => {

test/unit/plugin/retryFailedStep_test.js

@@ -142,4 +142,21 @@ describe('retryFailedStep', () => {
     // expects to retry only once
     counter.should.equal(2);
   });
+
+  it('should not turn around the chain of retries', () => {
+    recorder.retry({ retries: 2, when: (err) => { return err.message === 'someerror'; }, identifier: 'test' });
+    recorder.retry({ retries: 2, when: (err) => { return err.message === 'othererror'; } });
+
+    const getRetryIndex = () => recorder.retries.indexOf(recorder.retries.find(retry => retry.identifier));
+    let initalIndex;
+
+    recorder.add(() => {
+      initalIndex = getRetryIndex();
+    }, undefined, undefined, true);
+
+    recorder.add(() => {
+      initalIndex.should.equal(getRetryIndex());
+    }, undefined, undefined, true);
+    return recorder.promise();
+  });
 });