Return meaningful JSON RPC errors (#1213) (#1234)

ebadiere · georgi-l95 · web-flow · commit 433a5e5a15c3 · 2023-05-10T16:07:15.000-06:00
* return error msg



* fix tests and code smells



* fix tests



* handle msg in error class



---------

Signed-off-by: georgi-l95 &lt;glazarov95@gmail.com&gt;
Signed-off-by: ebadiere &lt;ebadiere@gmail.com&gt;
Co-authored-by: Georgi Lazarov &lt;glazarov95@gmail.com&gt;
diff --git a/packages/relay/src/lib/errors/JsonRpcError.ts b/packages/relay/src/lib/errors/JsonRpcError.ts
@@ -74,7 +74,7 @@ export const predefined = {
   'INTERNAL_ERROR': (message = '') => new JsonRpcError({
     name: 'Internal error',
     code: -32603,
-    message: message === '' ? 'Unknown error invoking RPC' : `Error invoking RPC: ${message}`
+    message: message === '' || undefined ? 'Unknown error invoking RPC' : `Error invoking RPC: ${message}`
   }),
   'INVALID_PARAMETER': (index: number | string, message: string) =>  new JsonRpcError({
     name: 'Invalid parameter',
diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -997,7 +997,7 @@ export class EthImpl implements Eth {
       if (e instanceof JsonRpcError) {
         return e;
       }
-      return predefined.INTERNAL_ERROR();
+      return predefined.INTERNAL_ERROR(e.message.toString());
     }
   }
 
@@ -1056,7 +1056,7 @@ export class EthImpl implements Eth {
       if (e instanceof JsonRpcError) {
         return e;
       }
-      return predefined.INTERNAL_ERROR();
+      return predefined.INTERNAL_ERROR(e.message.toString());
     }
   }
 
@@ -1107,7 +1107,10 @@ export class EthImpl implements Eth {
       return await this.callConsensusNode(call, gas, requestId);
     } catch (e: any) {
       this.logger.error(e, `${requestIdPrefix} Failed to successfully submit eth_call`);
-      return e instanceof JsonRpcError ? e : predefined.INTERNAL_ERROR();
+      if (e instanceof JsonRpcError) {
+        return e;
+      }
+      return predefined.INTERNAL_ERROR(e.message.toString());
     }
   }
 
@@ -1139,7 +1142,10 @@ export class EthImpl implements Eth {
         return await this.callConsensusNode(call, gas, requestId);
       } 
       this.logger.error(e, `${requestIdPrefix} Failed to successfully submit eth_call`);
-      return e instanceof JsonRpcError ? e : predefined.INTERNAL_ERROR();
+      if (e instanceof JsonRpcError) {
+        return e;
+      }
+      return predefined.INTERNAL_ERROR(e.message.toString());
     }
   }
 
@@ -1179,7 +1185,7 @@ export class EthImpl implements Eth {
       if (e instanceof JsonRpcError) {
         return e;
       }
-      return predefined.INTERNAL_ERROR();
+      return predefined.INTERNAL_ERROR(e.message.toString());
     }
   }
 
@@ -1586,8 +1592,7 @@ export class EthImpl implements Eth {
           e,
           `${requestIdPrefix} Failed to retrieve contract result details for contract address ${to} at timestamp=${timestamp}`
         );
-
-        throw predefined.INTERNAL_ERROR();
+        throw predefined.INTERNAL_ERROR(e.message.toString());
       });
   }
 
@@ -1756,8 +1761,7 @@ export class EthImpl implements Eth {
     if (error instanceof JsonRpcError) {
       throw error;
     }
-
-    return predefined.INTERNAL_ERROR();
+    return predefined.INTERNAL_ERROR(error.message.toString());
   }
 
   /**************************************************
diff --git a/packages/relay/tests/lib/eth.spec.ts b/packages/relay/tests/lib/eth.spec.ts
@@ -3475,8 +3475,8 @@ describe('Eth calls using MirrorNode', async function () {
         await ethImpl.sendRawTransaction(txHash);
       } catch (e) {
         hasError = true;
-        expect(e.code).to.equal(predefined.INTERNAL_ERROR().code);
-        expect(e.message).to.equal(predefined.INTERNAL_ERROR().message);
+        expect(e.code).to.equal(predefined.INTERNAL_ERROR(e.message).code);
+        expect(`Error invoking RPC: ${e.message}`).to.equal(predefined.INTERNAL_ERROR(e.message).message);
       }
       expect(hasError).to.be.true;
     });
diff --git a/packages/server/tests/acceptance/rpc_batch1.spec.ts b/packages/server/tests/acceptance/rpc_batch1.spec.ts
@@ -498,6 +498,24 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
                 Assertions.transactionReceipt(res, mirrorResult);
             });
 
+            it('@release should fail to execute "eth_getTransactionReceipt" for hash of London transaction', async function () {
+                const gasPrice = await relay.gasPrice(requestId);
+                const transaction = {
+                    ...defaultLondonTransactionData,
+                    to: mirrorContract.evm_address,
+                    nonce: await relay.getAccountNonce('0x' + accounts[2].address, requestId),
+                    maxFeePerGas: gasPrice,
+                    maxPriorityFeePerGas: gasPrice
+                };
+
+                const signedTx = await accounts[2].wallet.signTransaction(transaction);
+                try {
+                    await relay.sendRawTransaction(signedTx+"11", requestId);
+                } catch (error) {
+                    expect(`Error invoking RPC: ${error.message}`).to.equal(predefined.INTERNAL_ERROR(error.message).message);
+                } 
+            });
+
             it('should execute "eth_getTransactionReceipt" for non-existing hash', async function () {
                 const res = await relay.call('eth_getTransactionReceipt', [NON_EXISTING_TX_HASH], requestId);
                 expect(res).to.be.null;
@@ -584,7 +602,11 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
                     gasPrice: await relay.gasPrice(requestId)
                 };
                 const signedTx = await accounts[2].wallet.signTransaction(transaction);
-                await relay.callFailing('eth_sendRawTransaction', [signedTx], predefined.INTERNAL_ERROR(), requestId);
+                try {
+                    await relay.sendRawTransaction(signedTx, requestId);
+                } catch (error) {
+                    expect(`Error invoking RPC: ${error.message}`).to.equal(predefined.INTERNAL_ERROR(error.message).message);
+                } 
             });
 
             it('should fail "eth_sendRawTransaction" for Legacy 2930 transactions (with gas price too low)', async function () {
diff --git a/packages/server/tests/clients/relayClient.ts b/packages/server/tests/clients/relayClient.ts
@@ -61,6 +61,9 @@ export default class RelayClient {
             this.logger.trace(`${requestIdPrefix} [POST] to relay '${methodName}' with params [${params}] returned ${JSON.stringify(res)}`);
             Assertions.expectedError();
         } catch (err) {
+            if (expectedRpcError.name == "Internal error"){
+                expectedRpcError = predefined.INTERNAL_ERROR(err.message);
+            }
             Assertions.jsonRpcError(err, expectedRpcError);
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -997,7 +997,7 @@ export class EthImpl implements Eth {`
`997`	`997`	`if (e instanceof JsonRpcError) {`
`998`	`998`	`return e;`
`999`	`999`	`}`
`1000`		`- return predefined.INTERNAL_ERROR();`
	`1000`	`+ return predefined.INTERNAL_ERROR(e.message.toString());`
`1001`	`1001`	`}`
`1002`	`1002`	`}`
`1003`	`1003`
`@@ -1056,7 +1056,7 @@ export class EthImpl implements Eth {`
`1056`	`1056`	`if (e instanceof JsonRpcError) {`
`1057`	`1057`	`return e;`
`1058`	`1058`	`}`
`1059`		`- return predefined.INTERNAL_ERROR();`
	`1059`	`+ return predefined.INTERNAL_ERROR(e.message.toString());`
`1060`	`1060`	`}`
`1061`	`1061`	`}`
`1062`	`1062`
`@@ -1107,7 +1107,10 @@ export class EthImpl implements Eth {`
`1107`	`1107`	`return await this.callConsensusNode(call, gas, requestId);`
`1108`	`1108`	`} catch (e: any) {`
`1109`	`1109`	this.logger.error(e, `${requestIdPrefix} Failed to successfully submit eth_call`);
`1110`		`- return e instanceof JsonRpcError ? e : predefined.INTERNAL_ERROR();`
	`1110`	`+ if (e instanceof JsonRpcError) {`
	`1111`	`+ return e;`
	`1112`	`+ }`
	`1113`	`+ return predefined.INTERNAL_ERROR(e.message.toString());`
`1111`	`1114`	`}`
`1112`	`1115`	`}`
`1113`	`1116`
`@@ -1139,7 +1142,10 @@ export class EthImpl implements Eth {`
`1139`	`1142`	`return await this.callConsensusNode(call, gas, requestId);`
`1140`	`1143`	`}`
`1141`	`1144`	this.logger.error(e, `${requestIdPrefix} Failed to successfully submit eth_call`);
`1142`		`- return e instanceof JsonRpcError ? e : predefined.INTERNAL_ERROR();`
	`1145`	`+ if (e instanceof JsonRpcError) {`
	`1146`	`+ return e;`
	`1147`	`+ }`
	`1148`	`+ return predefined.INTERNAL_ERROR(e.message.toString());`
`1143`	`1149`	`}`
`1144`	`1150`	`}`
`1145`	`1151`
`@@ -1179,7 +1185,7 @@ export class EthImpl implements Eth {`
`1179`	`1185`	`if (e instanceof JsonRpcError) {`
`1180`	`1186`	`return e;`
`1181`	`1187`	`}`
`1182`		`- return predefined.INTERNAL_ERROR();`
	`1188`	`+ return predefined.INTERNAL_ERROR(e.message.toString());`
`1183`	`1189`	`}`
`1184`	`1190`	`}`
`1185`	`1191`
`@@ -1586,8 +1592,7 @@ export class EthImpl implements Eth {`
`1586`	`1592`	`e,`
`1587`	`1593`	`${requestIdPrefix} Failed to retrieve contract result details for contract address ${to} at timestamp=${timestamp}`
`1588`	`1594`	`);`
`1589`		`-`
`1590`		`- throw predefined.INTERNAL_ERROR();`
	`1595`	`+ throw predefined.INTERNAL_ERROR(e.message.toString());`
`1591`	`1596`	`});`
`1592`	`1597`	`}`
`1593`	`1598`
`@@ -1756,8 +1761,7 @@ export class EthImpl implements Eth {`
`1756`	`1761`	`if (error instanceof JsonRpcError) {`
`1757`	`1762`	`throw error;`
`1758`	`1763`	`}`
`1759`		`-`
`1760`		`- return predefined.INTERNAL_ERROR();`
	`1764`	`+ return predefined.INTERNAL_ERROR(error.message.toString());`
`1761`	`1765`	`}`
`1762`	`1766`
`1763`	`1767`	`/**************************************************`
Original file line number	Diff line number	Diff line change
`@@ -61,6 +61,9 @@ export default class RelayClient {`
`61`	`61`	this.logger.trace(`${requestIdPrefix} [POST] to relay '${methodName}' with params [${params}] returned ${JSON.stringify(res)}`);
`62`	`62`	`Assertions.expectedError();`
`63`	`63`	`} catch (err) {`
	`64`	`+ if (expectedRpcError.name == "Internal error"){`
	`65`	`+ expectedRpcError = predefined.INTERNAL_ERROR(err.message);`
	`66`	`+ }`
`64`	`67`	`Assertions.jsonRpcError(err, expectedRpcError);`
`65`	`68`	`}`
`66`	`69`	`}`