Add tests for retryOnEmpty middleware (#17364)

* Add tests for `retryOnEmpty` middleware

Tests have been added for the `retryOnEmpty` middleware.

This middleware is only used on the Infura network client, so the tests
that demonstrate this retry behavior are only enabled for the `infura`
provider type.

Most of the tests added were to cover cases where the retry middleware
is skipped, so they were applicable for both provider types.

Closes #17004

* Improve readability of block number tests

The test cases for passing a block number parameter have been made more
readable. Specifically, a comment has been added each time params are
built to call attention to the block parameter and what value it has,
so that it's clear whether it's larger or smaller than the current
block number.

Additionally the blocks for "less than the current block number" and
"equal to the current block number" have been combined using
`describe.each`.

app/scripts/controllers/network/provider-api-tests/shared-tests.js

@@ -572,9 +572,31 @@ export function testsForRpcMethodAssumingNoBlockParam(
     });
   });
 
-  it.each([null, undefined, '\u003cnil\u003e'])(
+  for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
-    'does not reuse the result of a previous request if it was `%s`',
+    it(`does not retry an empty response of "${emptyValue}"`, async () => {
-    async (emptyValue) => {
+      const request = { method };
+      const mockResult = emptyValue;
+
+      await withMockedCommunications({ providerType }, async (comms) => {
+        // The first time a block-cacheable request is made, the latest block
+        // number is retrieved through the block tracker first. It doesn't
+        // matter what this is — it's just used as a cache key.
+        comms.mockNextBlockTrackerRequest();
+        comms.mockRpcCall({
+          request,
+          response: { result: mockResult },
+        });
+
+        const result = await withNetworkClient(
+          { providerType },
+          ({ makeRpcCall }) => makeRpcCall(request),
+        );
+
+        expect(result).toStrictEqual(mockResult);
+      });
+    });
+
+    it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => {
       const requests = [{ method }, { method }];
       const mockResults = [emptyValue, 'some result'];
 
@@ -599,8 +621,8 @@ export function testsForRpcMethodAssumingNoBlockParam(
 
         expect(results).toStrictEqual(mockResults);
       });
-    },
+    });
-  );
+  }
 
   it('queues requests while a previous identical call is still pending, then runs the queue when it finishes, reusing the result from the first request', async () => {
     const requests = [{ method }, { method }, { method }];
@@ -1504,9 +1526,31 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse(
     });
   });
 
-  it.each([null, undefined, '\u003cnil\u003e'])(
+  for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
-    'does not reuse the result of a previous request if it was `%s`',
+    it(`does not retry an empty response of "${emptyValue}"`, async () => {
-    async (emptyValue) => {
+      const request = { method };
+      const mockResult = emptyValue;
+
+      await withMockedCommunications({ providerType }, async (comms) => {
+        // The first time a block-cacheable request is made, the latest block
+        // number is retrieved through the block tracker first. It doesn't
+        // matter what this is — it's just used as a cache key.
+        comms.mockNextBlockTrackerRequest();
+        comms.mockRpcCall({
+          request,
+          response: { result: mockResult },
+        });
+
+        const result = await withNetworkClient(
+          { providerType },
+          ({ makeRpcCall }) => makeRpcCall(request),
+        );
+
+        expect(result).toStrictEqual(mockResult);
+      });
+    });
+
+    it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => {
       const requests = [{ method }, { method }];
       const mockResults = [emptyValue, { blockHash: '0x100' }];
 
@@ -1531,8 +1575,8 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse(
 
         expect(results).toStrictEqual(mockResults);
       });
-    },
+    });
-  );
+  }
 
   it('does not reuse the result of a previous request if result.blockHash was null', async () => {
     const requests = [{ method }, { method }];
@@ -1800,9 +1844,44 @@ export function testsForRpcMethodSupportingBlockParam(
       });
     });
 
-    it.each([null, undefined, '\u003cnil\u003e'])(
+    for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
-      'does not reuse the result of a previous request if it was `%s`',
+      it(`does not retry an empty response of "${emptyValue}"`, async () => {
-      async (emptyValue) => {
+        const request = {
+          method,
+          params: buildMockParams({ blockParamIndex, blockParam }),
+        };
+        const mockResult = emptyValue;
+
+        await withMockedCommunications({ providerType }, async (comms) => {
+          // The first time a block-cacheable request is made, the
+          // block-cache middleware will request the latest block number
+          // through the block tracker to determine the cache key. Later,
+          // the block-ref middleware will request the latest block number
+          // again to resolve the value of "latest", but the block number is
+          // cached once made, so we only need to mock the request once.
+          comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' });
+          // The block-ref middleware will make the request as specified
+          // except that the block param is replaced with the latest block
+          // number.
+          comms.mockRpcCall({
+            request: buildRequestWithReplacedBlockParam(
+              request,
+              blockParamIndex,
+              '0x100',
+            ),
+            response: { result: mockResult },
+          });
+
+          const result = await withNetworkClient(
+            { providerType },
+            ({ makeRpcCall }) => makeRpcCall(request),
+          );
+
+          expect(result).toStrictEqual(mockResult);
+        });
+      });
+
+      it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => {
         const requests = [
           { method, params: buildMockParams({ blockParamIndex, blockParam }) },
           { method, params: buildMockParams({ blockParamIndex, blockParam }) },
@@ -1844,8 +1923,8 @@ export function testsForRpcMethodSupportingBlockParam(
 
           expect(results).toStrictEqual(mockResults);
         });
-      },
+      });
-    );
+    }
 
     it('queues requests while a previous identical call is still pending, then runs the queue when it finishes, reusing the result from the first request', async () => {
       const requests = [{ method }, { method }, { method }];
@@ -3095,9 +3174,67 @@ export function testsForRpcMethodSupportingBlockParam(
       });
     });
 
-    it.each([null, undefined, '\u003cnil\u003e'])(
+    if (blockParamType === 'earliest') {
-      'does not reuse the result of a previous request if it was `%s`',
+      it('treats "0x00" as a synonym for "earliest"', async () => {
-      async (emptyValue) => {
+        const requests = [
+          {
+            method,
+            params: buildMockParams({ blockParamIndex, blockParam }),
+          },
+          {
+            method,
+            params: buildMockParams({ blockParamIndex, blockParam: '0x00' }),
+          },
+        ];
+        const mockResults = ['first result', 'second result'];
+
+        await withMockedCommunications({ providerType }, async (comms) => {
+          // The first time a block-cacheable request is made, the latest
+          // block number is retrieved through the block tracker first. It
+          // doesn't matter what this is — it's just used as a cache key.
+          comms.mockNextBlockTrackerRequest();
+          comms.mockRpcCall({
+            request: requests[0],
+            response: { result: mockResults[0] },
+          });
+
+          const results = await withNetworkClient(
+            { providerType },
+            ({ makeRpcCallsInSeries }) => makeRpcCallsInSeries(requests),
+          );
+
+          expect(results).toStrictEqual([mockResults[0], mockResults[0]]);
+        });
+      });
+
+      for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
+        it(`does not retry an empty response of "${emptyValue}"`, async () => {
+          const request = {
+            method,
+            params: buildMockParams({ blockParamIndex, blockParam }),
+          };
+          const mockResult = emptyValue;
+
+          await withMockedCommunications({ providerType }, async (comms) => {
+            // The first time a block-cacheable request is made, the latest block
+            // number is retrieved through the block tracker first. It doesn't
+            // matter what this is — it's just used as a cache key.
+            comms.mockNextBlockTrackerRequest();
+            comms.mockRpcCall({
+              request,
+              response: { result: mockResult },
+            });
+
+            const result = await withNetworkClient(
+              { providerType },
+              ({ makeRpcCall }) => makeRpcCall(request),
+            );
+
+            expect(result).toStrictEqual(mockResult);
+          });
+        });
+
+        it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => {
           const requests = [
             {
               method,
@@ -3131,42 +3268,9 @@ export function testsForRpcMethodSupportingBlockParam(
 
             expect(results).toStrictEqual(mockResults);
           });
-      },
-    );
-
-    if (blockParamType === 'earliest') {
-      it('treats "0x00" as a synonym for "earliest"', async () => {
-        const requests = [
-          {
-            method,
-            params: buildMockParams({ blockParamIndex, blockParam }),
-          },
-          {
-            method,
-            params: buildMockParams({ blockParamIndex, blockParam: '0x00' }),
-          },
-        ];
-        const mockResults = ['first result', 'second result'];
-
-        await withMockedCommunications({ providerType }, async (comms) => {
-          // The first time a block-cacheable request is made, the latest
-          // block number is retrieved through the block tracker first. It
-          // doesn't matter what this is — it's just used as a cache key.
-          comms.mockNextBlockTrackerRequest();
-          comms.mockRpcCall({
-            request: requests[0],
-            response: { result: mockResults[0] },
-          });
-
-          const results = await withNetworkClient(
-            { providerType },
-            ({ makeRpcCallsInSeries }) => makeRpcCallsInSeries(requests),
-          );
-
-          expect(results).toStrictEqual([mockResults[0], mockResults[0]]);
-        });
         });
       }
+    }
 
     if (blockParamType === 'block number') {
       it('does not reuse the result of a previous request if it was made with different arguments than this one', async () => {
@@ -3204,18 +3308,26 @@ export function testsForRpcMethodSupportingBlockParam(
         });
       });
 
-      it('makes an additional request to the RPC endpoint if the given block number matches the latest block number', async () => {
+      describe.each(
+        [
+          ['less than the current block number', '0x200'],
+          ['equal to the curent block number', '0x100'],
+        ],
+        '%s',
+        (_nestedDesc, currentBlockNumber) => {
+          it('makes an additional request to the RPC endpoint', async () => {
             await withMockedCommunications({ providerType }, async (comms) => {
               const request = {
                 method,
-            params: buildMockParams({ blockParamIndex, blockParam: '0x100' }),
+                // Note that `blockParam` is `0x100` here
+                params: buildMockParams({ blockParamIndex, blockParam }),
               };
 
               // The first time a block-cacheable request is made, the latest
-          // block number is retrieved through the block tracker first. This
+              // block number is retrieved through the block tracker first.
-          // also happens within the retry-on-empty middleware (although the
+              comms.mockNextBlockTrackerRequest({
-          // latest block is cached by now).
+                blockNumber: currentBlockNumber,
-          comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' });
+              });
               comms.mockRpcCall({
                 request,
                 response: { result: 'the result' },
@@ -3230,18 +3342,187 @@ export function testsForRpcMethodSupportingBlockParam(
             });
           });
 
-      it('makes an additional request to the RPC endpoint if the given block number is less than the latest block number', async () => {
+          for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
+            if (providerType === 'infura') {
+              it(`retries up to 10 times if a "${emptyValue}" response is returned, returning successful non-empty response if there is one on the 10th try`, async () => {
+                const request = {
+                  method,
+                  // Note that `blockParam` is `0x100` here
+                  params: buildMockParams({ blockParamIndex, blockParam }),
+                };
+
+                await withMockedCommunications(
+                  { providerType },
+                  async (comms) => {
+                    // The first time a block-cacheable request is made, the latest block
+                    // number is retrieved through the block tracker first.
+                    comms.mockNextBlockTrackerRequest({
+                      blockNumber: currentBlockNumber,
+                    });
+                    comms.mockRpcCall({
+                      request,
+                      response: { result: emptyValue },
+                      times: 9,
+                    });
+                    comms.mockRpcCall({
+                      request,
+                      response: { result: 'some value' },
+                    });
+
+                    const result = await withNetworkClient(
+                      { providerType },
+                      ({ makeRpcCall, clock }) =>
+                        waitForPromiseToBeFulfilledAfterRunningAllTimers(
+                          makeRpcCall(request),
+                          clock,
+                        ),
+                    );
+
+                    expect(result).toStrictEqual('some value');
+                  },
+                );
+              });
+
+              it(`retries up to 10 times if a "${emptyValue}" response is returned, failing after the 10th try`, async () => {
+                const request = {
+                  method,
+                  // Note that `blockParam` is `0x100` here
+                  params: buildMockParams({ blockParamIndex, blockParam }),
+                };
+                const mockResult = emptyValue;
+
+                await withMockedCommunications(
+                  { providerType },
+                  async (comms) => {
+                    // The first time a block-cacheable request is made, the latest block
+                    // number is retrieved through the block tracker first.
+                    comms.mockNextBlockTrackerRequest({
+                      blockNumber: currentBlockNumber,
+                    });
+                    comms.mockRpcCall({
+                      request,
+                      response: { result: mockResult },
+                      times: 10,
+                    });
+
+                    const promiseForResult = withNetworkClient(
+                      { providerType },
+                      ({ makeRpcCall, clock }) =>
+                        waitForPromiseToBeFulfilledAfterRunningAllTimers(
+                          makeRpcCall(request),
+                          clock,
+                        ),
+                    );
+
+                    await expect(promiseForResult).rejects.toThrow(
+                      'RetryOnEmptyMiddleware - retries exhausted',
+                    );
+                  },
+                );
+              });
+            } else {
+              it(`does not retry an empty response of "${emptyValue}"`, async () => {
+                const request = {
+                  method,
+                  // Note that `blockParam` is `0x100` here
+                  params: buildMockParams({ blockParamIndex, blockParam }),
+                };
+                const mockResult = emptyValue;
+
+                await withMockedCommunications(
+                  { providerType },
+                  async (comms) => {
+                    // The first time a block-cacheable request is made, the latest block
+                    // number is retrieved through the block tracker first.
+                    comms.mockNextBlockTrackerRequest({
+                      blockNumber: currentBlockNumber,
+                    });
+                    comms.mockRpcCall({
+                      request: buildRequestWithReplacedBlockParam(
+                        request,
+                        blockParamIndex,
+                        '0x100',
+                      ),
+                      response: { result: mockResult },
+                    });
+
+                    const result = await withNetworkClient(
+                      { providerType },
+                      ({ makeRpcCall }) => makeRpcCall(request),
+                    );
+
+                    expect(result).toStrictEqual(mockResult);
+                  },
+                );
+              });
+
+              it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => {
+                const requests = [
+                  {
+                    method,
+                    // Note that `blockParam` is `0x100` here
+                    params: buildMockParams({ blockParamIndex, blockParam }),
+                  },
+                  {
+                    method,
+                    // Note that `blockParam` is `0x100` here
+                    params: buildMockParams({ blockParamIndex, blockParam }),
+                  },
+                ];
+                const mockResults = [emptyValue, { blockHash: '0x100' }];
+
+                await withMockedCommunications(
+                  { providerType },
+                  async (comms) => {
+                    // The first time a block-cacheable request is made, the latest block
+                    // number is retrieved through the block tracker first.
+                    comms.mockNextBlockTrackerRequest({
+                      blockNumber: currentBlockNumber,
+                    });
+                    comms.mockRpcCall({
+                      request: buildRequestWithReplacedBlockParam(
+                        requests[0],
+                        blockParamIndex,
+                        '0x100',
+                      ),
+                      response: { result: mockResults[0] },
+                    });
+                    comms.mockRpcCall({
+                      request: buildRequestWithReplacedBlockParam(
+                        requests[1],
+                        blockParamIndex,
+                        '0x100',
+                      ),
+                      response: { result: mockResults[1] },
+                    });
+
+                    const results = await withNetworkClient(
+                      { providerType },
+                      ({ makeRpcCallsInSeries }) =>
+                        makeRpcCallsInSeries(requests),
+                    );
+
+                    expect(results).toStrictEqual(mockResults);
+                  },
+                );
+              });
+            }
+          }
+        },
+      );
+
+      describe('greater than the current block number', () => {
+        it('makes an additional request to the RPC endpoint', async () => {
           await withMockedCommunications({ providerType }, async (comms) => {
             const request = {
               method,
-            params: buildMockParams({ blockParamIndex, blockParam: '0x50' }),
+              // Note that `blockParam` is `0x100` here
+              params: buildMockParams({ blockParamIndex, blockParam }),
             };
 
             // The first time a block-cacheable request is made, the latest
-          // block number is retrieved through the block tracker first. This
+            // block number is retrieved through the block tracker first.
-          // also happens within the retry-on-empty middleware (although the
+            comms.mockNextBlockTrackerRequest({ blockNumber: '0x42' });
-          // latest block is cached by now).
-          comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' });
             comms.mockRpcCall({
               request,
               response: { result: 'the result' },
@@ -3255,6 +3536,84 @@ export function testsForRpcMethodSupportingBlockParam(
             expect(result).toStrictEqual('the result');
           });
         });
+
+        for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
+          it(`does not retry an empty response of "${emptyValue}"`, async () => {
+            const request = {
+              method,
+              // Note that `blockParam` is `0x100` here
+              params: buildMockParams({ blockParamIndex, blockParam }),
+            };
+            const mockResult = emptyValue;
+
+            await withMockedCommunications({ providerType }, async (comms) => {
+              // The first time a block-cacheable request is made, the latest block
+              // number is retrieved through the block tracker first.
+              comms.mockNextBlockTrackerRequest({ blockNumber: '0x42' });
+              comms.mockRpcCall({
+                request: buildRequestWithReplacedBlockParam(
+                  request,
+                  blockParamIndex,
+                  '0x100',
+                ),
+                response: { result: mockResult },
+              });
+
+              const result = await withNetworkClient(
+                { providerType },
+                ({ makeRpcCall }) => makeRpcCall(request),
+              );
+
+              expect(result).toStrictEqual(mockResult);
+            });
+          });
+
+          it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => {
+            const requests = [
+              {
+                method,
+                // Note that `blockParam` is `0x100` here
+                params: buildMockParams({ blockParamIndex, blockParam }),
+              },
+              {
+                method,
+                // Note that `blockParam` is `0x100` here
+                params: buildMockParams({ blockParamIndex, blockParam }),
+              },
+            ];
+            const mockResults = [emptyValue, { blockHash: '0x100' }];
+
+            await withMockedCommunications({ providerType }, async (comms) => {
+              // The first time a block-cacheable request is made, the latest block
+              // number is retrieved through the block tracker first.
+              comms.mockNextBlockTrackerRequest({ blockNumber: '0x42' });
+              comms.mockRpcCall({
+                request: buildRequestWithReplacedBlockParam(
+                  requests[0],
+                  blockParamIndex,
+                  '0x100',
+                ),
+                response: { result: mockResults[0] },
+              });
+              comms.mockRpcCall({
+                request: buildRequestWithReplacedBlockParam(
+                  requests[1],
+                  blockParamIndex,
+                  '0x100',
+                ),
+                response: { result: mockResults[1] },
+              });
+
+              const results = await withNetworkClient(
+                { providerType },
+                ({ makeRpcCallsInSeries }) => makeRpcCallsInSeries(requests),
+              );
+
+              expect(results).toStrictEqual(mockResults);
+            });
+          });
+        }
+      });
     }
   });