Mark PendingTransactionTracker#resubmitPendingTxs as async (#8399)

app/scripts/controllers/transactions/pending-tx-tracker.js

@@ -52,45 +52,41 @@ export default class PendingTransactionTracker extends EventEmitter {
    * Resubmits each pending transaction
    * @param {string} blockNumber - the latest block number in hex
    * @emits tx:warning
+   * @returns {Promise<void>}
    */
-  resubmitPendingTxs (blockNumber) {
+  async resubmitPendingTxs (blockNumber) {
     const pending = this.getPendingTransactions()
-    // only try resubmitting if their are transactions to resubmit
     if (!pending.length) {
       return
     }
-    pending.forEach((txMeta) => this._resubmitTx(txMeta, blockNumber).catch((err) => {
+    for (const txMeta of pending) {
-      /*
+      try {
-      Dont marked as failed if the error is a "known" transaction warning
+        await this._resubmitTx(txMeta, blockNumber)
-      "there is already a transaction with the same sender-nonce
+      } catch (err) {
-      but higher/same gas price"
+        const errorMessage = err.message.toLowerCase()
-
+        const isKnownTx = (
-      Also don't mark as failed if it has ever been broadcast successfully.
+          // geth
-      A successful broadcast means it may still be mined.
+          errorMessage.includes('replacement transaction underpriced') ||
-      */
+          errorMessage.includes('known transaction') ||
-      const errorMessage = err.message.toLowerCase()
+          // parity
-      const isKnownTx = (
+          errorMessage.includes('gas price too low to replace') ||
-        // geth
+          errorMessage.includes('transaction with the same hash was already imported') ||
-        errorMessage.includes('replacement transaction underpriced') ||
+          // other
-        errorMessage.includes('known transaction') ||
+          errorMessage.includes('gateway timeout') ||
-        // parity
+          errorMessage.includes('nonce too low')
-        errorMessage.includes('gas price too low to replace') ||
+        )
-        errorMessage.includes('transaction with the same hash was already imported') ||
+        // ignore resubmit warnings, return early
-        // other
+        if (isKnownTx) {
-        errorMessage.includes('gateway timeout') ||
+          return
-        errorMessage.includes('nonce too low')
+        }
-      )
+        // encountered real error - transition to error state
-      // ignore resubmit warnings, return early
+        txMeta.warning = {
-      if (isKnownTx) {
+          error: errorMessage,
-        return
+          message: 'There was an error when resubmitting this transaction.',
+        }
+        this.emit('tx:warning', txMeta, err)
       }
-      // encountered real error - transition to error state
+    }
-      txMeta.warning = {
-        error: errorMessage,
-        message: 'There was an error when resubmitting this transaction.',
-      }
-      this.emit('tx:warning', txMeta, err)
-    }))
   }
 
   /**

test/unit/app/controllers/transactions/pending-tx-tracker-test.js

@@ -25,7 +25,7 @@ describe('PendingTransactionTracker', function () {
       const warningListener = sinon.spy()
 
       pendingTxTracker.on('tx:warning', warningListener)
-      pendingTxTracker.resubmitPendingTxs('0x1')
+      await pendingTxTracker.resubmitPendingTxs('0x1')
 
       assert.ok(getPendingTransactions.calledOnceWithExactly(), 'should call getPendingTransaction')
       assert.ok(resubmitTx.notCalled, 'should NOT call _resubmitTx')
@@ -57,7 +57,7 @@ describe('PendingTransactionTracker', function () {
       const warningListener = sinon.spy()
 
       pendingTxTracker.on('tx:warning', warningListener)
-      pendingTxTracker.resubmitPendingTxs('0x1')
+      await pendingTxTracker.resubmitPendingTxs('0x1')
 
       assert.ok(getPendingTransactions.calledOnceWithExactly(), 'should call getPendingTransaction')
       assert.ok(resubmitTx.calledTwice, 'should call _resubmitTx')
@@ -87,12 +87,42 @@ describe('PendingTransactionTracker', function () {
       const warningListener = sinon.spy()
 
       pendingTxTracker.on('tx:warning', warningListener)
-      pendingTxTracker.resubmitPendingTxs('0x1')
+      await pendingTxTracker.resubmitPendingTxs('0x1')
 
       assert.ok(getPendingTransactions.calledOnceWithExactly(), 'should call getPendingTransaction')
       assert.ok(resubmitTx.calledOnce, 'should call _resubmitTx')
       assert.ok(warningListener.notCalled, "should NOT emit 'tx:warning'")
     })
+
+    it("should emit 'tx:warning' for unknown failed resubmission", async function () {
+      const getPendingTransactions = sinon.stub().returns([{
+        id: 1,
+      }])
+      const pendingTxTracker = new PendingTransactionTracker({
+        query: {
+          getTransactionReceipt: sinon.stub(),
+        },
+        nonceTracker: {
+          getGlobalLock: sinon.stub().resolves({
+            releaseLock: sinon.spy(),
+          }),
+        },
+        getPendingTransactions,
+        getCompletedTransactions: sinon.stub().returns([]),
+        approveTransaction: sinon.spy(),
+        publishTransaction: sinon.spy(),
+        confirmTransaction: sinon.spy(),
+      })
+      const resubmitTx = sinon.stub(pendingTxTracker, '_resubmitTx').rejects({ message: 'who dis' })
+      const warningListener = sinon.spy()
+
+      pendingTxTracker.on('tx:warning', warningListener)
+      await pendingTxTracker.resubmitPendingTxs('0x1')
+
+      assert.ok(getPendingTransactions.calledOnceWithExactly(), 'should call getPendingTransaction')
+      assert.ok(resubmitTx.calledOnce, 'should call _resubmitTx')
+      assert.ok(warningListener.calledOnce, "should emit 'tx:warning'")
+    })
   })
 
   describe('#updatePendingTxs', function () {