fix(replybot): clear stale error/wait/retries on state transitions

replybot/lib/typewheels/machine.js

@@ -579,7 +579,7 @@ function apply(state, output) {
 
     case 'RESPOND':
 
-      // NOTE: by removing errors/retries on RESPOND, we are "resetting"
+      // NOTE: by removing errors/retries/waits on RESPOND, we are "resetting"
       // our retry-on-error process (and exponential backoff) whenever
       // the user responds. I think this is reasonable. But it's implicit here.
       return {
@@ -591,6 +591,8 @@ function apply(state, output) {
         previousOutput: output,
         error: undefined, // remove error when responding
         retries: undefined, // remove retries when responding
+        wait: undefined, // remove wait when user responds
+        waitStart: undefined, // remove waitStart when user responds
         qa: updateQA(state.qa, update(output))
       }
 
@@ -610,7 +612,9 @@ function apply(state, output) {
       return {
         ...state,
         ...output.stateUpdate,
-        state: 'RESPONDING'
+        state: 'RESPONDING',
+        error: undefined, // remove stale error on retry (keep retries for backoff)
+        wait: undefined, // remove stale wait on retry
       }
 
 
@@ -628,7 +632,9 @@ function apply(state, output) {
       return {
         ...state,
         state: 'QOUT',
-        question: output.question
+        question: output.question,
+        error: undefined, // question sent, no error context
+        retries: undefined, // question sent, no retry context
       }
 
     case 'HANDOFF':
@@ -639,7 +645,9 @@ function apply(state, output) {
         question: output.question,
         wait: output.wait,
         externalEvents: output.externalEvents || state.externalEvents,
-        waitStart: output.waitStart
+        waitStart: output.waitStart,
+        error: undefined, // entering wait, clear prior error
+        retries: undefined, // entering wait, clear prior retries
       }
 
     case 'WAIT_EXTERNAL_EVENT':
@@ -650,21 +658,42 @@ function apply(state, output) {
         question: output.question,
         wait: output.wait,
         externalEvents: output.externalEvents || state.externalEvents,
-        waitStart: output.waitStart
+        waitStart: output.waitStart,
+        error: undefined, // entering/continuing wait, clear prior error
+        retries: undefined, // entering/continuing wait, clear prior retries
       }
 
 
     case 'END':
-      return { ...state, state: 'END', question: output.question }
+      return {
+        ...state,
+        state: 'END',
+        question: output.question,
+        error: undefined, // completed, no error context
+        wait: undefined, // completed, no wait context
+        retries: undefined, // completed, no retry context
+      }
 
     case 'BLOCKED':
-      return { ...state, state: 'BLOCKED', error: output.error }
+      return {
+        ...state,
+        state: 'BLOCKED',
+        error: output.error,
+        wait: undefined, // blocked, clear prior wait
+        waitStart: undefined, // blocked, clear prior waitStart
+      }
 
     case 'UNBLOCK':
       return { ...state, ...output.stateUpdate }
 
     case 'ERROR':
-      return { ...state, state: 'ERROR', error: output.error }
+      return {
+        ...state,
+        state: 'ERROR',
+        error: output.error,
+        wait: undefined, // errored, clear prior wait
+        waitStart: undefined, // errored, clear prior waitStart
+      }
 
     default:
       return state

replybot/lib/typewheels/machine.test.js

@@ -451,7 +451,7 @@ describe('getState', () => {
   })
 
 
-  it('Responds while waiting with response and repeats with old waitstart', () => {
+  it('Responds while waiting with response and gets fresh waitstart on new wait', () => {
 
     const wait = { type: 'timeout', value: '2 days' }
 
@@ -464,7 +464,7 @@ describe('getState', () => {
     ]
     const state = getState(log)
     state.state.should.equal('WAIT_EXTERNAL_EVENT')
-    state.waitStart.should.equal(5)
+    state.waitStart.should.equal(10)
   })
 
 
@@ -827,6 +827,69 @@ describe('getState', () => {
     actions.messages[0].recipient.one_time_notif_token.should.equal('FOOBAR')
     actions.messages[0].message.text.should.equal('barbaz')
   })
+
+  describe('transient field cleanup', () => {
+
+    it('clears error on REDO but keeps retries accumulating', () => {
+      const report = synthetic({ type: 'machine_report', value: { error: { tag: 'INTERNAL', code: 'FOO' } } })
+      const redo = synthetic({ type: 'redo' })
+      const log = [referral, echo, text, report, redo]
+      const state = getState(log)
+      state.state.should.equal('RESPONDING')
+      should.not.exist(state.error)
+      state.retries.should.exist
+      state.retries.length.should.equal(1)
+    })
+
+    it('clears error and retries when user responds from ERROR state', () => {
+      const report = synthetic({ type: 'machine_report', value: { error: { tag: 'INTERNAL', code: 'FOO' } } })
+      const log = [referral, echo, text, report, text]
+      const state = getState(log)
+      state.state.should.equal('RESPONDING')
+      should.not.exist(state.error)
+      should.not.exist(state.retries)
+    })
+
+    it('clears wait when entering ERROR from WAIT_EXTERNAL_EVENT', () => {
+      const wait = { type: 'timeout', value: '2 days' }
+      const report = synthetic({ type: 'machine_report', value: { error: { tag: 'INTERNAL', code: 'FOO' } } })
+      const log = [referral, echo, text, _echo({ wait, ref: 'bar' }), report]
+      const state = getState(log)
+      state.state.should.equal('ERROR')
+      should.not.exist(state.wait)
+      should.not.exist(state.waitStart)
+      state.error.code.should.equal('FOO')
+    })
+
+    it('clears wait when entering BLOCKED from WAIT_EXTERNAL_EVENT', () => {
+      const wait = { type: 'timeout', value: '2 days' }
+      const fbReport = synthetic({ type: 'machine_report', value: { error: { tag: 'FB', code: 200, message: 'foo' } } })
+      const log = [referral, echo, text, _echo({ wait, ref: 'bar' }), fbReport]
+      const state = getState(log)
+      state.state.should.equal('BLOCKED')
+      should.not.exist(state.wait)
+      should.not.exist(state.waitStart)
+    })
+
+    it('clears error, wait, and retries on END', () => {
+      const report = synthetic({ type: 'machine_report', value: { error: { tag: 'INTERNAL', code: 'FOO' } } })
+      const log = [referral, echo, text, report, tyEcho]
+      const state = getState(log)
+      state.state.should.equal('END')
+      should.not.exist(state.error)
+      should.not.exist(state.wait)
+      should.not.exist(state.retries)
+    })
+
+    it('clears wait and waitStart when user responds while waiting', () => {
+      const wait = { type: 'timeout', value: '2 days' }
+      const log = [referral, echo, delivery, text, _echo({ wait, ref: 'bar' }), text]
+      const state = getState(log)
+      state.state.should.equal('RESPONDING')
+      should.not.exist(state.wait)
+      should.not.exist(state.waitStart)
+    })
+  })
 })