Abstract error handling for addresses into AbstractService (#842)

* Abstract error handling for addresses into AbstractService

* Add yarn cache, jest cov and fixed unit test

.github/workflows/pr.yml

@@ -9,28 +9,37 @@ on:
       - master
 
 jobs:
-  lint:
+  check:
+    # The type of runner that the job will run on
     runs-on: ubuntu-latest
-    steps:
-    - name: Install wasm-pack
-      uses: jetli/wasm-pack-action@v0.3.0
-      with:
-        version: v0.10.0
-    - uses: actions/checkout@v2
-    - name: Lint
-      run: |
-        yarn install
-        yarn lint
 
-  test:
-    runs-on: ubuntu-latest
+    # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
-    - name: Install wasm-pack
-      uses: jetli/wasm-pack-action@v0.3.0
-      with:
-        version: v0.10.0
-    - uses: actions/checkout@v2
-    - name: Test
-      run: |
-        yarn install
-        yarn test
+      - name: Checkout files
+        uses: actions/checkout@v2
+
+      - name: Install Node v16
+        uses: actions/setup-node@v2
+        with:
+          node-version: '16.13'
+
+      - name: Get yarn cache directory path
+        id: yarn-cache-dir-path
+        run: echo "::set-output name=dir::$(yarn config get cacheFolder)"
+
+      - uses: actions/cache@v2
+        id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`)
+        with:
+          path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
+          key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-yarn-
+      
+      - name: Install JS dependencies
+        run: yarn install
+
+      - name: Linter.
+        run: yarn lint:ci
+
+      - name: Unit tests Cov.
+        run: yarn test:cov

.gitignore

@@ -1,4 +1,3 @@
-/.vscode
 **/node_modules
 /build
 /e2e-tests/build
@@ -17,3 +16,23 @@ yarn-error.log
 !.yarn/releases
 !.yarn/plugins
 /docs/.yarn
+
+# Tests
+/coverage
+/.nyc_output
+
+# IDEs and editors
+/.idea
+.project
+.classpath
+.c9/
+*.launch
+.settings/
+*.sublime-workspace
+
+# IDE - VSCode
+.vscode/*
+!.vscode/settings.json
+!.vscode/tasks.json
+!.vscode/launch.json
+!.vscode/extensions.json

docs/src/openapi-v1.yaml

@@ -67,6 +67,12 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/AccountAssetsBalances'
+        "400":
+          description: Invalid Address
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
   /accounts/{accountId}/asset-approvals:
     get:
       tags:
@@ -115,6 +121,12 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/AccountAssetsApproval'
+        "400":
+          description: Invalid Address
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
   /accounts/{accountId}/balance-info:
     get:
       tags:
@@ -158,7 +170,7 @@ paths:
               schema:
                 $ref: '#/components/schemas/AccountBalanceInfo'
         "400":
-          description: invalid blockId supplied for at query param
+          description: Invalid Address
           content:
             application/json:
               schema:
@@ -313,7 +325,7 @@ paths:
               schema:
                 $ref: '#/components/schemas/AccountVestingInfo'
         "400":
-          description: invalid blockId supplied for at query param
+          description: Invalid Address
           content:
             application/json:
               schema:

jest.config.js

@@ -2,5 +2,15 @@ const base = require('@substrate/dev/config/jest')
 
 module.exports = {
 	...base,
+	verbose: true,
+	coverageThreshold: {
+		global: {
+			branches: 50,
+			functions: 70,
+			lines: 70,
+			statements: 85
+		}
+	},
+	testEnvironment: 'node',
 	testPathIgnorePatterns: ['/build/', '/node_modules/', '/docs/', '/e2e-tests/'],
 };

package.json

@@ -28,12 +28,15 @@
     "build:docker": "docker build -t substrate-api-sidecar .",
     "build:docs": "(cd docs && yarn && yarn build)",
     "main": "node ./build/src/main.js",
-    "lint": "substrate-dev-run-lint",
+    "lint": "substrate-dev-run-lint --fix",
+    "lint:ci": "substrate-dev-run-lint",
     "deploy": "yarn build && npm publish",
     "start": "yarn run main",
     "start:log-rpc": "yarn run build && NODE_ENV=test yarn run main ",
     "dev": "tsc-watch --onSuccess \"yarn run main\"",
-    "test": "substrate-exec-jest --silent",
+    "test": "substrate-exec-jest",
+    "test:watch": "substrate-exec-jest --watch",
+    "test:cov": "substrate-exec-jest --coverage",
     "lint:e2e-tests": "cd e2e-tests && substrate-dev-run-lint",
     "build:e2e-tests": "(cd e2e-tests && substrate-exec-tsc)",
     "test:e2e-tests": "yarn build:e2e-tests && node ./e2e-tests/build/index.js --config=./e2e-tests/jest.config.js",

src/services/AbstractService.spec.ts

@@ -0,0 +1,62 @@
+import { ApiPromise } from '@polkadot/api';
+import { Hash } from '@polkadot/types/interfaces';
+import { BadRequest, HttpError } from 'http-errors';
+
+import {
+	AbstractService,
+	EtheuremAddressNotSupported,
+} from './AbstractService';
+import { defaultMockApi } from './test-helpers/mock';
+
+class TestAbstractService extends AbstractService {
+	public handleEtheuremAddressError(): HttpError {
+		return this.createHttpErrorForAddr(
+			'0x0000000000000000000000000000000000000000',
+			new Error('Ups! something is wrong.')
+		);
+	}
+
+	public handlePolkadotAddressError(): HttpError {
+		return this.createHttpErrorForAddr(
+			'EQBwtmKWCyRrQ8yGWg7LkB8p7hpEKXZz4qUg9WR8hZmieCM',
+			new Error('Ups! something is wrong.')
+		);
+	}
+}
+
+const mockApi = {
+	...defaultMockApi,
+	at: (_hash: Hash) => ({}),
+} as unknown as ApiPromise;
+
+const testService = new TestAbstractService(mockApi);
+
+describe('AbstractService', () => {
+	describe('createHttpErrorForAddr', () => {
+		it('should throws an error instanceof EtheuremAddressNotSupported', () => {
+			const err = testService.handleEtheuremAddressError();
+
+			expect(err.expose).toBeTruthy();
+			expect(err.headers).toBeUndefined();
+			expect(err.message).not.toBeUndefined();
+			expect(err.name).toEqual(EtheuremAddressNotSupported.name);
+			expect(err.stack).not.toBeUndefined();
+			expect(err.status).toEqual(400);
+			expect(err.statusCode).toEqual(400);
+			expect(err).toBeInstanceOf(EtheuremAddressNotSupported);
+		});
+
+		it('should throws an error instanceof BadRequest', () => {
+			const err = testService.handlePolkadotAddressError();
+
+			expect(err.expose).toBeTruthy();
+			expect(err.headers).toBeUndefined();
+			expect(err.message).not.toBeUndefined();
+			expect(err.name).toEqual(BadRequest.name);
+			expect(err.stack).not.toBeUndefined();
+			expect(err.status).toEqual(400);
+			expect(err.statusCode).toEqual(400);
+			expect(err).toBeInstanceOf(BadRequest);
+		});
+	});
+});

src/services/AbstractService.ts

@@ -1,5 +1,21 @@
 import { ApiPromise } from '@polkadot/api';
 import { Text, Vec } from '@polkadot/types';
+import { isEthereumAddress } from '@polkadot/util-crypto';
+import { BadRequest, HttpError } from 'http-errors';
+
+export class EtheuremAddressNotSupported extends Error implements HttpError {
+	public readonly status: number;
+	public readonly statusCode: number;
+	public readonly expose: boolean = true;
+	public readonly name: string;
+
+	constructor(msg: string) {
+		super(`Etheurem addresses may not be supported on this network: ${msg}`);
+		this.status = 400;
+		this.statusCode = 400;
+		this.name = EtheuremAddressNotSupported.name;
+	}
+}
 
 export abstract class AbstractService {
 	constructor(protected api: ApiPromise) {}
@@ -16,4 +32,20 @@ export abstract class AbstractService {
 			)
 			.join('');
 	}
+
+	/**
+	 * Returns HttpError with the correct err message for querying accounts balances.
+	 *
+	 * @function
+	 * @param {string} address Address that was queried
+	 * @param {Error} err Error returned from the promise
+	 * @returns {HttpError}
+	 */
+	protected createHttpErrorForAddr(address: string, err: Error): HttpError {
+		if (isEthereumAddress(address)) {
+			return new EtheuremAddressNotSupported(err.message);
+		}
+
+		return new BadRequest(err.message);
+	}
 }

src/services/accounts/AccountsAssetsService.ts

@@ -113,7 +113,9 @@ export class AccountsAssetsService extends AbstractService {
 		const [{ number }, assetApproval] = await Promise.all([
 			api.rpc.chain.getHeader(hash),
 			historicApi.query.assets.approvals(assetId, address, delegate),
-		]);
+		]).catch((err: Error) => {
+			throw this.createHttpErrorForAddr(address, err);
+		});
 
 		let amount = null,
 			deposit = null;
@@ -211,7 +213,9 @@ export class AccountsAssetsService extends AbstractService {
 					isSufficient: historicApi.registry.createType('bool', false),
 				};
 			})
-		);
+		).catch((err: Error) => {
+			throw this.createHttpErrorForAddr(address, err);
+		});
 	}
 
 	/**

src/services/accounts/AccountsBalanceInfoService.ts

@@ -7,8 +7,7 @@ import {
 	BlockHash,
 	Index,
 } from '@polkadot/types/interfaces';
-import { isEthereumAddress } from '@polkadot/util-crypto';
-import { BadRequest, HttpError } from 'http-errors';
+import { BadRequest } from 'http-errors';
 import { IAccountBalanceInfo } from 'src/types/responses';
 
 import { AbstractService } from '../AbstractService';
@@ -47,7 +46,7 @@ export class AccountsBalanceInfoService extends AbstractService {
 				historicApi.query.balances.reservedBalance(address) as Promise<Balance>,
 				historicApi.query.system.accountNonce(address) as Promise<Index>,
 			]).catch((err: Error) => {
-				throw this.createHttpError(address, err);
+				throw this.createHttpErrorForAddr(address, err);
 			});
 
 			// Values dont exist for these historic runtimes
@@ -79,7 +78,7 @@ export class AccountsBalanceInfoService extends AbstractService {
 				historicApi.query.system.account(address),
 				historicApi.query.balances.locks(address),
 			]).catch((err: Error) => {
-				throw this.createHttpError(address, err);
+				throw this.createHttpErrorForAddr(address, err);
 			});
 
 			const {
@@ -118,7 +117,7 @@ export class AccountsBalanceInfoService extends AbstractService {
 				historicApi.query.balances.locks(address),
 				historicApi.query.system.account(address),
 			]).catch((err: Error) => {
-				throw this.createHttpError(address, err);
+				throw this.createHttpErrorForAddr(address, err);
 			});
 
 			accountData =
@@ -172,18 +171,4 @@ export class AccountsBalanceInfoService extends AbstractService {
 			throw new BadRequest('Account not found');
 		}
 	}
-
-	/**
-	 * Returns HttpError with the correct err message for querying accounts balances.
-	 *
-	 * @param address Address that was queried
-	 * @param err Error returned from the promise
-	 */
-	private createHttpError(address: string, err: Error): HttpError {
-		return isEthereumAddress(address)
-			? new BadRequest(
-					`Etheurem addresses may not be supported on this network: ${err.message}`
-			  )
-			: new BadRequest(err.message);
-	}
 }

src/services/accounts/AccountsStakingInfoService.ts

@@ -21,7 +21,9 @@ export class AccountsStakingInfoService extends AbstractService {
 		const [header, controllerOption] = await Promise.all([
 			api.rpc.chain.getHeader(hash),
 			historicApi.query.staking.bonded(stash), // Option<AccountId> representing the controller
-		]);
+		]).catch((err: Error) => {
+			throw this.createHttpErrorForAddr(stash, err);
+		});
 
 		const at = {
 			hash,
@@ -39,7 +41,9 @@ export class AccountsStakingInfoService extends AbstractService {
 				historicApi.query.staking.ledger(controller),
 				historicApi.query.staking.payee(stash),
 				historicApi.query.staking.slashingSpans(stash),
-			]);
+			]).catch((err: Error) => {
+				throw this.createHttpErrorForAddr(stash, err);
+			});
 
 		const stakingLedger = stakingLedgerOption.unwrapOr(null);
 

src/services/accounts/AccountsStakingPayoutsService.ts

@@ -116,7 +116,9 @@ export class AccountsStakingPayoutsService extends AbstractService {
 			startEra,
 			// Create an array of `DeriveEraExposure`
 			allErasGeneral.map((eraGeneral) => eraGeneral[0])
-		);
+		).catch((err: Error) => {
+			throw this.createHttpErrorForAddr(address, err);
+		});
 
 		// Group together data by Era so we can easily associate parts that are used congruently downstream
 		const allEraData = allErasGeneral.map(

src/services/accounts/AccountsVestingInfoService.ts

@@ -28,7 +28,9 @@ export class AccountsVestingInfoService extends AbstractService {
 		const [{ number }, vesting] = await Promise.all([
 			api.rpc.chain.getHeader(hash),
 			historicApi.query.vesting.vesting(address),
-		]);
+		]).catch((err: Error) => {
+			throw this.createHttpErrorForAddr(address, err);
+		});
 
 		const at = {
 			hash,