Improve error handling for eth_getLogs (#388)

Ivo-Yankov · web-flow · commit f23e77b471f4 · 2022-07-28T10:13:09.000+03:00
* fix: improve eth_getLogs error handling

Signed-off-by: Ivo Yankov &lt;ivo@devlabs.bg&gt;

* wip: fix failing tests

Signed-off-by: Ivo Yankov &lt;ivo@devlabs.bg&gt;

* chore: resolve code structure conflicts

Signed-off-by: Ivo Yankov &lt;ivo@devlabs.bg&gt;

* test: assert the errors

Signed-off-by: Ivo Yankov &lt;ivo@devlabs.bg&gt;

* nit: fix code smells

Signed-off-by: Ivo Yankov &lt;ivo@devlabs.bg&gt;
diff --git a/packages/relay/src/index.ts b/packages/relay/src/index.ts
@@ -19,9 +19,10 @@
  */
 
 import { Block, Log, Receipt, Transaction } from './lib/model';
-import { JsonRpcError } from './lib/errors/JsonRpcError';
+import { JsonRpcError, predefined } from './lib/errors/JsonRpcError';
+import { MirrorNodeClientError } from './lib/errors/MirrorNodeClientError';
 
-export { JsonRpcError };
+export { JsonRpcError, predefined, MirrorNodeClientError };
 
 export { RelayImpl } from './lib/relay';
 
diff --git a/packages/relay/src/lib/errors/JsonRpcError.ts b/packages/relay/src/lib/errors/JsonRpcError.ts
@@ -106,4 +106,9 @@ export const predefined = {
     code: -32602,
     message: 'Value below 10_000_000_000 wei which is 1 tinybar'
   }),
+  'REQUEST_TIMEOUT': new JsonRpcError({
+    name: 'Request timeout',
+    code: -32010,
+    message: 'Request timeout. Please try again.'
+  }),
 };
diff --git a/packages/relay/src/lib/errors/MirrorNodeClientError.ts b/packages/relay/src/lib/errors/MirrorNodeClientError.ts
@@ -21,12 +21,25 @@
 
 export class MirrorNodeClientError extends Error {
     public statusCode: number;
+
+    static statusCodes = {
+        TIMEOUT: 567,
+        NOT_FOUND: 404
+    };
   
     constructor(message: string, statusCode: number) {
       super(message);
       this.statusCode = statusCode;
   
       Object.setPrototypeOf(this, MirrorNodeClientError.prototype);
     }
+
+    public isTimeout(): boolean {
+        return this.statusCode === MirrorNodeClientError.statusCodes.TIMEOUT;
+    }
+
+    public isNotFound(): boolean {
+        return this.statusCode === MirrorNodeClientError.statusCodes.NOT_FOUND;
+    }
   }
   
diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -26,6 +26,7 @@ import { Block, Transaction, Log } from './model';
 import { MirrorNodeClient, SDKClient } from './clients';
 import { JsonRpcError, predefined } from './errors/JsonRpcError';
 import { SDKClientError } from './errors/SDKClientError';
+import { MirrorNodeClientError } from './errors/MirrorNodeClientError';
 import constants from './constants';
 import { Precheck } from './precheck';
 
@@ -1040,28 +1041,36 @@ export class EthImpl implements Eth {
   async getLogs(blockHash: string | null, fromBlock: string | null, toBlock: string | null, address: string | null, topics: any[] | null): Promise<Log[]> {
     const params: any = {};
     if (blockHash) {
-      const block = await this.mirrorNodeClient.getBlock(blockHash);
-      if (block) {
-        params.timestamp = [
-          `gte:${block.timestamp.from}`,
-          `lte:${block.timestamp.to}`
-        ];
+      try {
+        const block = await this.mirrorNodeClient.getBlock(blockHash);
+        if (block) {
+          params.timestamp = [
+            `gte:${block.timestamp.from}`,
+            `lte:${block.timestamp.to}`
+          ];
+        }
       }
-    }
-    else if (fromBlock || toBlock) {
+      catch(e: any) {
+        if (e instanceof MirrorNodeClientError && e.isNotFound()) {
+          return [];
+        }
+
+        throw e;
+      }
+    } else if (fromBlock || toBlock) {
       const filters = [];
       let order;
-      if(toBlock) {
+      if (toBlock) {
         // @ts-ignore
         filters.push(`lte:${parseInt(toBlock)}`);
         order = constants.ORDER.DESC;
       }
-      if(fromBlock) {
+      if (fromBlock) {
         // @ts-ignore
         filters.push(`gte:${parseInt(fromBlock)}`);
         order = constants.ORDER.ASC;
       }
-      const blocksResult = await this.mirrorNodeClient.getBlocks(filters, undefined , {order});
+      const blocksResult = await this.mirrorNodeClient.getBlocks(filters, undefined, {order});
 
       const blocks = blocksResult?.blocks;
       if (blocks?.length) {
@@ -1112,8 +1121,8 @@ export class EthImpl implements Eth {
       }
     }
 
-    // Populate the Log objects with block and transaction data from ContractResultsDetails
     try {
+      // Populate the Log objects with block and transaction data from ContractResultsDetails
       const contractsResultsDetails = await Promise.all(promises);
       for (let i = 0; i < contractsResultsDetails.length; i++) {
         const detail = contractsResultsDetails[i];
@@ -1137,8 +1146,12 @@ export class EthImpl implements Eth {
         }
       }
     }
-    catch (e) {
-      return [];
+    catch(e: any) {
+      if (e instanceof MirrorNodeClientError && e.isNotFound()) {
+        return [];
+      }
+
+      throw e;
     }
 
     return logs;
diff --git a/packages/relay/tests/lib/eth.spec.ts b/packages/relay/tests/lib/eth.spec.ts
@@ -27,7 +27,7 @@ import { Registry } from 'prom-client';
 import sinon from 'sinon';
 import cache from 'js-cache';
 dotenv.config({ path: path.resolve(__dirname, '../test.env') });
-import { RelayImpl } from '@hashgraph/json-rpc-relay';
+import {RelayImpl, predefined, MirrorNodeClientError} from '@hashgraph/json-rpc-relay';
 import { EthImpl } from '../../src/lib/eth';
 import { MirrorNodeClient } from '../../src/lib/clients/mirrorNodeClient';
 import { expectUnsupportedMethod } from '../helpers';
@@ -337,6 +337,7 @@ describe('Eth calls using MirrorNode', async function () {
   };
 
   const detailedContractResultNotFound = { "_status": { "messages": [{ "message": "No correlating transaction" }] } };
+  const timeoutError = { "type": "Error", "message": "timeout of 10000ms exceeded" };
 
   const defaultDetailedContractResultsWithNullNullableValues = {
     ...defaultDetailedContractResults,
@@ -1016,6 +1017,51 @@ describe('Eth calls using MirrorNode', async function () {
       expect(result.length).to.eq(0);
     });
 
+    it('one of contract results details timeouts and throws the expected error', async function () {
+      mock.onGet(`contracts/results/logs`).reply(200, defaultLogs);
+      mock.onGet(`contracts/${contractId1}/results/${contractTimestamp1}`).reply(200, defaultDetailedContractResults);
+      mock.onGet(`contracts/${contractId1}/results/${contractTimestamp2}`).reply(567, timeoutError);
+      mock.onGet(`contracts/${contractId2}/results/${contractTimestamp2}`).reply(200, defaultDetailedContractResults3);
+
+      try {
+        const result = await ethImpl.getLogs(null, null, null, null, null);
+        expect(true).to.eq(false);
+      } catch (error: any) {
+        expect(error.statusCode).to.equal(MirrorNodeClientError.statusCodes.TIMEOUT);
+      }
+    });
+
+    it('blockHash filter timeouts and throws the expected error', async function () {
+      const filteredLogs = {
+        logs: [defaultLogs.logs[0], defaultLogs.logs[1]]
+      };
+
+      mock.onGet(`contracts/results/logs`).reply(200, defaultLogs);
+      mock.onGet(`contracts/${contractId1}/results/${contractTimestamp1}`).reply(200, defaultDetailedContractResults);
+      mock.onGet(`contracts/results/logs?timestamp=gte:${defaultBlock.timestamp.from}&timestamp=lte:${defaultBlock.timestamp.to}`).reply(200, filteredLogs);
+      mock.onGet(`blocks/${blockHash}`).reply(567, timeoutError);
+
+      try {
+        const result = await ethImpl.getLogs(blockHash, null, null, null, null);
+        expect(true).to.eq(false);
+      } catch (error: any) {
+        expect(error.statusCode).to.equal(MirrorNodeClientError.statusCodes.TIMEOUT);
+      }
+    });
+
+    it('address filter timeouts and throws the expected error', async function () {
+      mock.onGet(`contracts/${contractId1}/results/${contractTimestamp1}`).reply(200, defaultDetailedContractResults);
+      mock.onGet(`contracts/${contractId1}/results/${contractTimestamp2}`).reply(200, defaultDetailedContractResults2);
+      mock.onGet(`contracts/${contractAddress1}/results/logs`).reply(567, timeoutError);
+
+      try {
+        const result = await ethImpl.getLogs(null, null, null, contractAddress1, null);
+        expect(true).to.eq(false);
+      } catch (error: any) {
+        expect(error.statusCode).to.equal(MirrorNodeClientError.statusCodes.TIMEOUT);
+      }
+    });
+
     it('error when retrieving logs', async function () {
       mock.onGet(`contracts/results/logs`).reply(400, { "_status": { "messages": [{ "message": "Mocked error" }] } });
       const result = await ethImpl.getLogs(null, null, null, null, null);
diff --git a/packages/server/src/server.ts b/packages/server/src/server.ts
@@ -18,7 +18,7 @@
  *
  */
 
-import { Relay, RelayImpl, JsonRpcError } from '@hashgraph/json-rpc-relay';
+import { Relay, RelayImpl, JsonRpcError, predefined, MirrorNodeClientError } from '@hashgraph/json-rpc-relay';
 import Koa from 'koa';
 import koaJsonRpc from 'koa-jsonrpc';
 import { collectDefaultMetrics, Histogram, Registry } from 'prom-client';
@@ -143,11 +143,21 @@ const logAndHandleResponse = async (methodName, methodFunction) => {
     methodResponseHistogram.labels(methodName, status).observe(ms);
     logger.info(`${messagePrefix} ${status} ${ms} ms `);
     return response;
-  } catch (e) {
+  } catch (e: any) {
     ms = Date.now() - start;
     methodResponseHistogram.labels(methodName, responseInternalErrorCode).observe(ms);
     logger.error(e, `${messagePrefix} ${responseInternalErrorCode} ${ms} ms`);
-    throw e;
+
+    if (e instanceof MirrorNodeClientError) {
+      if (e.isTimeout()) {
+        return predefined.REQUEST_TIMEOUT;
+      }
+    }
+    else if (e instanceof JsonRpcError) {
+      return e;
+    }
+
+    return predefined.INTERNAL_ERROR;
   }
 };
 
diff --git a/packages/server/tests/acceptance/rpc.spec.ts b/packages/server/tests/acceptance/rpc.spec.ts
@@ -209,6 +209,14 @@ describe('RPC Server Acceptance Tests', function () {
                 }
             });
 
+            it('should return empty result for  non-existing `blockHash`', async () => {
+                const logs = await relay.call('eth_getLogs', [{
+                    'blockHash': NON_EXISTING_BLOCK_HASH
+                }]);
+                expect(logs).to.exist;
+                expect(logs.length).to.be.eq(0);
+            });
+
             it('should be able to use `topics` param', async () => {
                 const logs = await relay.call('eth_getLogs', [{
                     'fromBlock': log0Block.blockNumber,
