refactor to address code review comments

Signed-off-by: lukelee-sl <[email protected]>
hashgraph · Jul 26, 2022 · eea0081 · eea0081
1 parent fb28bd3
commit eea0081
Show file tree

Hide file tree

Showing 12 changed files with 137 additions and 66 deletions.
diff --git a/packages/relay/src/index.ts b/packages/relay/src/index.ts
@@ -18,8 +18,8 @@
  *
  */
 
-import {Block, Log, Receipt, Transaction} from './lib/model';
-import {JsonRpcError} from './lib/errors';
+import { Block, Log, Receipt, Transaction } from './lib/model';
+import { JsonRpcError } from './lib/errors/JsonRpcError';
 
 export { JsonRpcError };
 

diff --git a/packages/relay/src/lib/clients/mirrorNodeClient.ts b/packages/relay/src/lib/clients/mirrorNodeClient.ts
@@ -19,7 +19,7 @@
  */
 
 import Axios, { AxiosInstance } from 'axios';
-import { MirrorNodeClientError } from '../errors';
+import { MirrorNodeClientError } from './../errors/MirrorNodeClientError';
 import { Logger } from "pino";
 import constants from './../constants';
 import { Histogram, Registry } from 'prom-client';

diff --git a/packages/relay/src/lib/clients/sdkClient.ts b/packages/relay/src/lib/clients/sdkClient.ts
@@ -45,7 +45,7 @@ import { BigNumber } from '@hashgraph/sdk/lib/Transfer';
 import { Logger } from "pino";
 import { Gauge, Histogram, Registry } from 'prom-client';
 import constants from './../constants';
-import { SDKClientError } from '../errors';
+import { SDKClientError } from './../errors/SDKClientError';
 
 const _ = require('lodash');
 
@@ -166,7 +166,7 @@ export class SDKClient {
     async getTinyBarGasFee(callerName: string): Promise<number> {
         const feeSchedules = await this.getFeeSchedule(callerName);
         if (_.isNil(feeSchedules.current) || feeSchedules.current?.transactionFeeSchedule === undefined) {
-            throw new SDKClientError('Invalid FeeSchedules proto format');
+            throw new SDKClientError({}, 'Invalid FeeSchedules proto format');
         }
 
         for (const schedule of feeSchedules.current?.transactionFeeSchedule) {
@@ -178,7 +178,7 @@ export class SDKClient {
             }
         }
 
-        throw new SDKClientError(`${constants.ETH_FUNCTIONALITY_CODE} code not found in feeSchedule`);
+        throw new SDKClientError({}, `${constants.ETH_FUNCTIONALITY_CODE} code not found in feeSchedule`);
     }
 
     async getFileIdBytes(address: string, callerName: string): Promise<Uint8Array> {
@@ -253,16 +253,18 @@ export class SDKClient {
             return resp;
         }
         catch (e: any) {
-            const statusCode = e.status ? e.status._code : Status.Unknown._code;
-            this.logger.debug(`Consensus Node query response: ${query.constructor.name} ${statusCode}`);
-            this.captureMetrics(
-                SDKClient.queryMode,
-                query.constructor.name,
-                e.status,
-                query._queryPayment?.toTinybars().toNumber(),
-                callerName);
+            const sdkClientError = new SDKClientError(e);
+            if(sdkClientError.isValidNetworkError()) {
+                this.logger.debug(`Consensus Node query response: ${query.constructor.name} ${sdkClientError.statusCode}`);
+                this.captureMetrics(
+                    SDKClient.queryMode,
+                    query.constructor.name,
+                    sdkClientError.status,
+                    query._queryPayment?.toTinybars().toNumber(),
+                    callerName);    
+            }
 
-            throw new SDKClientError(e.message, e?.status?._code);
+            throw sdkClientError;
         }
     };
 
@@ -275,23 +277,25 @@ export class SDKClient {
             return resp;
         }
         catch (e: any) {
-            const statusCode = e.status ? e.status._code : Status.Unknown._code;
-            this.logger.info(`Consensus Node ${transactionType} transaction response: ${statusCode}`);
-            this.captureMetrics(
-                SDKClient.transactionMode,
-                transactionType,
-                statusCode,
-                0,
-                callerName);
+            const sdkClientError = new SDKClientError(e);
+            if(sdkClientError.isValidNetworkError()) {
+                this.logger.info(`Consensus Node ${transactionType} transaction response: ${sdkClientError.statusCode}`);
+                this.captureMetrics(
+                    SDKClient.transactionMode,
+                    transactionType,
+                    sdkClientError.statusCode,
+                    0,
+                    callerName);
+            }
 
-            throw new SDKClientError(e.message, e?.status?._code);
+            throw sdkClientError;
         }
     };
 
     executeGetTransactionRecord = async (resp: TransactionResponse, transactionName: string, callerName: string): Promise<TransactionRecord> => {
         try {
             if (!resp.getRecord) {
-                throw new SDKClientError(`Invalid response format, expected record availability: ${JSON.stringify(resp)}`);
+                throw new SDKClientError({}, `Invalid response format, expected record availability: ${JSON.stringify(resp)}`);
             }
 
             const transactionRecord: TransactionRecord = await resp.getRecord(this.clientMain);
@@ -306,16 +310,17 @@ export class SDKClient {
         }
         catch (e: any) {
             // capture sdk record retrieval errors and shorten familiar stack trace
-            if (e.status && e.status._code) {
+            const sdkClientError = new SDKClientError(e);
+            if(sdkClientError.isValidNetworkError()) {
                 this.captureMetrics(
                     SDKClient.transactionMode,
                     transactionName,
-                    e.status,
+                    sdkClientError.status,
                     0,
                     callerName);
             }
 
-            throw new SDKClientError(e.message, e?.status?._code);
+            throw sdkClientError;
         }
     };
 

diff --git a/packages/relay/src/lib/errors.ts → ...ages/relay/src/lib/errors/JsonRpcError.ts b/packages/relay/src/lib/errors.ts → ...ages/relay/src/lib/errors/JsonRpcError.ts
@@ -18,8 +18,6 @@
  *
  */
 
-import { Status } from "@hashgraph/sdk";
-
 export class JsonRpcError {
   public code: number;
   public message: string;
@@ -108,28 +106,4 @@ export const predefined = {
     code: -32602,
     message: 'Value below 10_000_000_000 wei which is 1 tinybar'
   }),
-};
-
-export class SDKClientError extends Error {
-  public statusCode: number;
-
-  constructor(message: string, statusCode?: number) {
-    super(message);
-    this.statusCode = statusCode ? statusCode : Status.Unknown._code;
-    this.name = "SDKClientError";
-
-    Object.setPrototypeOf(this, SDKClientError.prototype);
-  }
-}
-
-export class MirrorNodeClientError extends Error {
-  public statusCode: number;
-
-  constructor(message: string, statusCode: number) {
-    super(message);
-    this.statusCode = statusCode;
-    this.name = "MirrorNodeClientError";
-
-    Object.setPrototypeOf(this, MirrorNodeClientError.prototype);
-  }
-}
+};
diff --git a/packages/relay/src/lib/errors/MirrorNodeClientError.ts b/packages/relay/src/lib/errors/MirrorNodeClientError.ts
@@ -0,0 +1,31 @@
+
+/*-
+ *
+ * Hedera JSON RPC Relay
+ *
+ * Copyright (C) 2022 Hedera Hashgraph, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+export class MirrorNodeClientError extends Error {
+    public statusCode: number;
+
+    constructor(message: string, statusCode: number) {
+      super(message);
+      this.statusCode = statusCode;
+
+      Object.setPrototypeOf(this, MirrorNodeClientError.prototype);
+    }
+  }
diff --git a/packages/relay/src/lib/errors/SDKClientError.ts b/packages/relay/src/lib/errors/SDKClientError.ts
@@ -0,0 +1,55 @@
+/*-
+ *
+ * Hedera JSON RPC Relay
+ *
+ * Copyright (C) 2022 Hedera Hashgraph, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+import { Status } from "@hashgraph/sdk";
+
+export class SDKClientError extends Error {
+  public status: Status = Status.Unknown;
+  private validNetworkError: boolean = false;
+
+  constructor(e: any, message?: string) {
+    super(e && e.status && e.status._code ? e.message : message);
+
+    if(e && e.status && e.status._code) {
+      this.validNetworkError = true;
+      this.status = e.status;
+    }
+
+    Object.setPrototypeOf(this, SDKClientError.prototype);
+  }
+
+  get statusCode(): number {
+    return this.status._code;
+  }
+
+  public isValidNetworkError(): boolean {
+    return this.validNetworkError;
+  }
+
+  public isInvalidAccountId(): boolean {
+    return this.isValidNetworkError() && this.statusCode === Status.InvalidAccountId._code;
+  }
+
+  public isInvalidContractId(): boolean {
+    return this.isValidNetworkError() && 
+      (this.statusCode === Status.InvalidContractId._code || this.message?.includes(Status.InvalidContractId.toString()));
+  }
+}
+
diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -24,7 +24,8 @@ import { BigNumber } from '@hashgraph/sdk/lib/Transfer';
 import { Logger } from 'pino';
 import { Block, Transaction, Log } from './model';
 import { MirrorNodeClient, SDKClient } from './clients';
-import { JsonRpcError, predefined, SDKClientError } from './errors';
+import { JsonRpcError, predefined } from './errors/JsonRpcError';
+import { SDKClientError } from './errors/SDKClientError';
 import constants from './constants';
 import { Precheck } from './precheck';
 
@@ -233,7 +234,7 @@ export class EthImpl implements Eth {
             'transaction_type': EthImpl.ethTxType
           }
         ]
-      };  
+      };
     }
 
     if (networkFees && Array.isArray(networkFees.fees)) {
@@ -456,7 +457,7 @@ export class EthImpl implements Eth {
     } catch (e: any) {
       if(e instanceof SDKClientError) {
         // handle INVALID_ACCOUNT_ID
-        if (e.statusCode === Status.InvalidAccountId._code) {
+        if (e.isInvalidAccountId()) {
           this.logger.debug(`Unable to find account ${account} in block ${JSON.stringify(blockNumber)}(${blockNumberOrTag}), returning 0x0 balance`);
           cache.set(cachedLabel, EthImpl.zeroHex, constants.CACHE_TTL.ONE_HOUR);
           return EthImpl.zeroHex;
@@ -490,7 +491,7 @@ export class EthImpl implements Eth {
     } catch (e: any) {
       if(e instanceof SDKClientError) {      
         // handle INVALID_CONTRACT_ID
-        if (e.statusCode === Status.InvalidContractId._code || e?.message?.includes(Status.InvalidContractId.toString())) {
+        if (e.isInvalidContractId()) {
           this.logger.debug('Unable to find code for contract %s in block "%s", returning 0x0, err code: %s', address, blockNumber, e.statusCode);
           cache.set(cachedLabel, '0x0', constants.CACHE_TTL.ONE_HOUR);
           return '0x0';

diff --git a/packages/relay/src/lib/precheck.ts b/packages/relay/src/lib/precheck.ts
@@ -19,7 +19,7 @@
  */
 
 import * as ethers from 'ethers';
-import { predefined } from './errors';
+import { predefined } from './errors/JsonRpcError';
 import { MirrorNodeClient, SDKClient } from './clients';
 import { EthImpl } from './eth';
 import { Logger } from 'pino';

diff --git a/packages/relay/tests/lib/eth.spec.ts b/packages/relay/tests/lib/eth.spec.ts
@@ -36,7 +36,7 @@ import pino from 'pino';
 import { Block, Transaction } from '../../src/lib/model';
 import constants from '../../src/lib/constants';
 import { SDKClient } from '../../src/lib/clients';
-import { SDKClientError } from '../../src/lib/errors';
+import { SDKClientError } from '../../src/lib/errors/SDKClientError';
 
 const logger = pino();
 const registry = new Registry();
@@ -904,7 +904,10 @@ describe('Eth calls using MirrorNode', async function () {
       mock.onGet(`accounts/${contractAddress1}`).reply(200, {
         account: contractAddress1
       });
-      sdkClientStub.getAccountBalanceInWeiBar.throws(new SDKClientError("eth_getBalance exception", 15));
+      sdkClientStub.getAccountBalanceInWeiBar.throws(new SDKClientError(
+        {status: {
+          _code: 15
+        }}));
 
       const resNoCache = await ethImpl.getBalance(contractAddress1, null);
       const resCached = await ethImpl.getBalance(contractAddress1, null);
@@ -947,7 +950,9 @@ describe('Eth calls using MirrorNode', async function () {
 
   describe('eth_getCode', async function() {
     it('should return cached value', async () => {
-      sdkClientStub.getContractByteCode.throws(new SDKClientError("eth_getBalance exception", 16));
+      sdkClientStub.getContractByteCode.throws(new SDKClientError({status: {
+        _code: 16
+      }}));
 
       const resNoCache = await ethImpl.getCode(contractAddress1, null);
       const resCached = await ethImpl.getCode(contractAddress1, null);

diff --git a/packages/server/tests/acceptance/rpc.spec.ts b/packages/server/tests/acceptance/rpc.spec.ts
@@ -30,7 +30,7 @@ import { AccountBalanceQuery, ContractFunctionParameters } from '@hashgraph/sdk'
 import parentContractJson from '../contracts/Parent.json';
 import basicContractJson from '../contracts/Basic.json';
 import logsContractJson from '../contracts/Logs.json';
-import { predefined } from '../../../relay/src/lib/errors';
+import { predefined } from '../../../relay/src/lib/errors/JsonRpcError';
 
 describe('RPC Server Acceptance Tests', function () {
     this.timeout(240 * 1000); // 240 seconds

diff --git a/packages/server/tests/clients/relayClient.ts b/packages/server/tests/clients/relayClient.ts
@@ -21,7 +21,7 @@
 import { ethers, providers } from 'ethers';
 import { Logger } from 'pino';
 import Assertions from '../helpers/assertions';
-import { predefined } from '../../../relay/src/lib/errors';
+import { predefined } from '../../../relay/src/lib/errors/JsonRpcError';
 
 export default class RelayClient {
 

diff --git a/packages/server/tests/helpers/assertions.ts b/packages/server/tests/helpers/assertions.ts
@@ -19,7 +19,7 @@
  */
 import { expect } from 'chai';
 import { ethers, BigNumber } from 'ethers';
-import { JsonRpcError, predefined } from '../../../relay/src/lib/errors';
+import { JsonRpcError, predefined } from '../../../relay/src/lib/errors/JsonRpcError';
 import { Utils } from './utils';
 
 export default class Assertions {