From 38362b3afe435387650320121f1417eb31a8a5fc Mon Sep 17 00:00:00 2001 From: fernando Date: Wed, 19 May 2021 19:04:00 -0300 Subject: [PATCH 1/3] fix: UnhandledPromiseRejection errors Simplified exception handling. Defer when loading node info metadata --unhandled-rejections=strict in tests Exception handling simplification --- package.json | 6 +- src/infrastructure/Http.ts | 65 ++++++++---------- src/infrastructure/RepositoryFactoryHttp.ts | 6 +- test/infrastructure/RepositoryFactory.spec.ts | 66 +++++++++++-------- 4 files changed, 74 insertions(+), 69 deletions(-) diff --git a/package.json b/package.json index 28d383096e..e0cad94d6a 100644 --- a/package.json +++ b/package.json @@ -5,11 +5,11 @@ "scripts": { "pretest": "npm run build", "prepack": "npm run build", - "test": "mocha --ui bdd --recursive ./dist/test --timeout 90000", + "test": "mocha --ui bdd --recursive ./dist/test --timeout 90000 --unhandled-rejections=strict ", "e2econfigcopy": "gulp", - "test:e2e": "npm run build && mocha --ui bdd --recursive ./dist/e2e --timeout 90000", + "test:e2e": "npm run build && mocha --ui bdd --recursive ./dist/e2e --timeout 90000 --unhandled-rejections=strict", "e2e": "npm run bootstrap-stop && npm run bootstrap-start-detached && npm run test:e2e && npm run bootstrap-stop", - "test:all": "mocha --ui bdd --recursive ./dist/ --timeout 90000", + "test:all": "mocha --ui bdd --recursive ./dist/ --timeout 90000 --unhandled-rejections=strict", "build": "shx rm -rf dist/ && tsc && npm run e2econfigcopy", "test:cov": "nyc --reporter=lcov --reporter=text-summary npm t", "test:coveralls": "npm run test:cov | coveralls", diff --git a/src/infrastructure/Http.ts b/src/infrastructure/Http.ts index 08568e9a25..4e6f4f9bac 100644 --- a/src/infrastructure/Http.ts +++ b/src/infrastructure/Http.ts @@ -15,10 +15,9 @@ */ import fetch from 'node-fetch'; -import { from as observableFrom, Observable, of as observableOf, of, throwError } from 'rxjs'; -import { catchError, flatMap, map } from 'rxjs/operators'; +import { defer, from as observableFrom, Observable, of as observableOf } from 'rxjs'; import { Configuration, NodeRoutesApi, Pagination, querystring } from 'symbol-openapi-typescript-fetch-client'; -import { NetworkType } from '../model/network/NetworkType'; +import { NetworkType } from '../model/network'; import { Page } from './Page'; import { RepositoryCallError } from './RepositoryCallError'; @@ -33,9 +32,9 @@ export abstract class Http { */ protected constructor(protected readonly url: string, protected readonly fetchApi?: any) {} - public static errorHandling(error: any): Observable { + public static async errorHandling(error: any): Promise { if (error instanceof Error) { - return throwError(error); + return error; } const statusCode: number = parseInt(error?.status || error?.statusCode || error?.response?.statusCode || 0); const statusMessage: string = ( @@ -55,29 +54,23 @@ export abstract class Http { return JSON.stringify(body); }; - const getBody = (error: any): Observable => { + const getBody = async (error: any): Promise => { const body = error?.response?.body; if (body) { - return of(toString(body)); + return toString(body); } if (error.text) { - return observableFrom(error.text()).pipe( - map(toString), - catchError(() => of('')), - ); + return toString(await error.text()); } - return of(''); + return ''; }; - return getBody(error).pipe( - flatMap((body: string) => { - const formattedError: RepositoryCallError = { - statusCode, - statusMessage, - body, - }; - return throwError(new Error(JSON.stringify(formattedError))); - }), - ); + + const formattedError: RepositoryCallError = { + statusCode, + statusMessage, + body: await getBody(error), + }; + return new Error(JSON.stringify(formattedError)); } createNetworkTypeObservable(networkType?: NetworkType | Observable): Observable { @@ -86,7 +79,7 @@ export abstract class Http { } else if (networkType) { return observableOf(networkType as NetworkType); } else { - return this.call(new NodeRoutesApi(this.config()).getNodeInfo(), (body) => body.networkIdentifier); + return defer(() => this.call(new NodeRoutesApi(this.config()).getNodeInfo(), (body) => body.networkIdentifier)); } } @@ -102,20 +95,18 @@ export abstract class Http { */ protected call(remoteCall: Promise, mapper: (value: D) => M): Observable { return observableFrom( - remoteCall.catch((e) => { - if (e instanceof Error) { - return Promise.resolve(e); - } - return Promise.reject(e); - }), - ).pipe( - map((body) => { - if (body instanceof Error) { - throw body; - } - return mapper(body); - }), - catchError(Http.errorHandling), + new Promise((resolve, reject) => + remoteCall.then( + async (a) => { + try { + resolve(mapper(a)); + } catch (e) { + reject(await Http.errorHandling(e)); + } + }, + async (e) => reject(await Http.errorHandling(e)), + ), + ), ); } diff --git a/src/infrastructure/RepositoryFactoryHttp.ts b/src/infrastructure/RepositoryFactoryHttp.ts index b46b93d160..8d752dc786 100644 --- a/src/infrastructure/RepositoryFactoryHttp.ts +++ b/src/infrastructure/RepositoryFactoryHttp.ts @@ -98,9 +98,9 @@ export class RepositoryFactoryHttp implements RepositoryFactory { this.generationHash = observableOf(configs.generationHash); this.nodePublicKey = observableOf(configs.nodePublicKey); } else { - const nodeInfoObservable = this.createNodeRepository().getNodeInfo(); - this.generationHash = this.cache(() => nodeInfoObservable.pipe(map((b) => b.networkGenerationHashSeed))); - this.nodePublicKey = this.cache(() => nodeInfoObservable.pipe(map((b) => b.nodePublicKey))); + const nodeInfoObservable = this.cache(() => this.createNodeRepository().getNodeInfo()); + this.generationHash = defer(() => nodeInfoObservable.pipe(map((b) => b.networkGenerationHashSeed))); + this.nodePublicKey = defer(() => nodeInfoObservable.pipe(map((b) => b.nodePublicKey))); } this.websocketUrl = configs?.websocketUrl ? configs?.websocketUrl : `${url.replace(/\/$/, '')}/ws`; this.websocketInjected = configs?.websocketInjected; diff --git a/test/infrastructure/RepositoryFactory.spec.ts b/test/infrastructure/RepositoryFactory.spec.ts index afe10a9eb2..0bf39a073d 100644 --- a/test/infrastructure/RepositoryFactory.spec.ts +++ b/test/infrastructure/RepositoryFactory.spec.ts @@ -18,34 +18,37 @@ import { of as observableOf, of } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; import { NetworkConfigurationDTO } from 'symbol-openapi-typescript-fetch-client'; import { instance, mock, when } from 'ts-mockito'; -import { AccountHttp } from '../../src/infrastructure/AccountHttp'; -import { BlockHttp } from '../../src/infrastructure/BlockHttp'; -import { ChainHttp } from '../../src/infrastructure/ChainHttp'; -import { FinalizationHttp } from '../../src/infrastructure/FinalizationHttp'; -import { HashLockHttp } from '../../src/infrastructure/HashLockHttp'; -import { Listener } from '../../src/infrastructure/Listener'; -import { MetadataHttp } from '../../src/infrastructure/MetadataHttp'; -import { MosaicHttp } from '../../src/infrastructure/MosaicHttp'; -import { MultisigHttp } from '../../src/infrastructure/MultisigHttp'; -import { NamespaceHttp } from '../../src/infrastructure/NamespaceHttp'; -import { NamespaceRepository } from '../../src/infrastructure/NamespaceRepository'; -import { NetworkHttp } from '../../src/infrastructure/NetworkHttp'; -import { NetworkRepository } from '../../src/infrastructure/NetworkRepository'; -import { NodeHttp } from '../../src/infrastructure/NodeHttp'; -import { NodeRepository } from '../../src/infrastructure/NodeRepository'; -import { ReceiptHttp } from '../../src/infrastructure/ReceiptHttp'; -import { RepositoryFactoryHttp } from '../../src/infrastructure/RepositoryFactoryHttp'; -import { RestrictionAccountHttp } from '../../src/infrastructure/RestrictionAccountHttp'; -import { RestrictionMosaicHttp } from '../../src/infrastructure/RestrictionMosaicHttp'; -import { SecretLockHttp } from '../../src/infrastructure/SecretLockHttp'; -import { TransactionGroup } from '../../src/infrastructure/TransactionGroup'; -import { TransactionHttp } from '../../src/infrastructure/TransactionHttp'; -import { TransactionStatusHttp } from '../../src/infrastructure/TransactionStatusHttp'; -import { NetworkCurrencies } from '../../src/model/mosaic/NetworkCurrencies'; -import { NetworkType } from '../../src/model/network/NetworkType'; -import { NodeInfo } from '../../src/model/node/NodeInfo'; +import { + AccountHttp, + BlockHttp, + ChainHttp, + FinalizationHttp, + HashLockHttp, + Listener, + MetadataHttp, + MosaicHttp, + MultisigHttp, + NamespaceHttp, + NamespaceRepository, + NetworkHttp, + NetworkRepository, + NodeHttp, + NodeRepository, + ReceiptHttp, + RepositoryFactoryHttp, + RestrictionAccountHttp, + RestrictionMosaicHttp, + SecretLockHttp, + TransactionGroup, + TransactionHttp, + TransactionStatusHttp, +} from '../../src/infrastructure'; +import { NetworkCurrencies } from '../../src/model/mosaic'; +import { NetworkType } from '../../src/model/network'; +import { NodeInfo } from '../../src/model/node'; describe('RepositoryFactory', () => { + it('Should create repositories', () => { const repositoryFactory = new RepositoryFactoryHttp('http://localhost:3000', { networkType: NetworkType.PRIVATE_TEST, @@ -70,6 +73,17 @@ describe('RepositoryFactory', () => { expect(repositoryFactory.createFinalizationRepository()).to.be.not.null; }); + it('Raise error without unhandled-rejections', async () => { + const url = 'https://www.google.com'; + const repositoryFactory = new RepositoryFactoryHttp(url); + try { + await repositoryFactory.createNodeRepository().getNodeHealth().toPromise(); + expect(true).to.be.false; + } catch (e) { + expect(e.message).contains('{"statusCode":404,"statusMessage":"Not Found","body":"'); + } + }); + it('Should get GenerationHash from cache', (done) => { let counter = 0; const repositoryMock: NodeRepository = mock(); From 5b6263848ff38c0f0381443d283ec004e10f0d02 Mon Sep 17 00:00:00 2001 From: fernando Date: Wed, 19 May 2021 19:54:58 -0300 Subject: [PATCH 2/3] fixed lint --- test/infrastructure/RepositoryFactory.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/infrastructure/RepositoryFactory.spec.ts b/test/infrastructure/RepositoryFactory.spec.ts index 0bf39a073d..3f247e06e7 100644 --- a/test/infrastructure/RepositoryFactory.spec.ts +++ b/test/infrastructure/RepositoryFactory.spec.ts @@ -48,7 +48,6 @@ import { NetworkType } from '../../src/model/network'; import { NodeInfo } from '../../src/model/node'; describe('RepositoryFactory', () => { - it('Should create repositories', () => { const repositoryFactory = new RepositoryFactoryHttp('http://localhost:3000', { networkType: NetworkType.PRIVATE_TEST, From b242a7e790631b5100f28b165f5ba22759810fda Mon Sep 17 00:00:00 2001 From: fernando Date: Fri, 21 May 2021 08:36:35 -0300 Subject: [PATCH 3/3] Feedback fixes --- test/infrastructure/RepositoryFactory.spec.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/infrastructure/RepositoryFactory.spec.ts b/test/infrastructure/RepositoryFactory.spec.ts index 3f247e06e7..21789f44c9 100644 --- a/test/infrastructure/RepositoryFactory.spec.ts +++ b/test/infrastructure/RepositoryFactory.spec.ts @@ -16,7 +16,7 @@ import { expect } from 'chai'; import { of as observableOf, of } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; -import { NetworkConfigurationDTO } from 'symbol-openapi-typescript-fetch-client'; +import { NetworkConfigurationDTO, NodeRoutesApi } from 'symbol-openapi-typescript-fetch-client'; import { instance, mock, when } from 'ts-mockito'; import { AccountHttp, @@ -73,13 +73,23 @@ describe('RepositoryFactory', () => { }); it('Raise error without unhandled-rejections', async () => { - const url = 'https://www.google.com'; + const nodeRoutesApi: NodeRoutesApi = mock(); + + const fetchResponseMock: Partial = { + status: 666, + statusText: 'Some status text error', + text: () => Promise.resolve('This is the body'), + }; + when(nodeRoutesApi.getNodeHealth()).thenReturn(Promise.reject(fetchResponseMock)); + const url = 'https://invalid'; const repositoryFactory = new RepositoryFactoryHttp(url); try { - await repositoryFactory.createNodeRepository().getNodeHealth().toPromise(); + const nodeRepository = repositoryFactory.createNodeRepository(); + (nodeRepository as any).nodeRoutesApi = instance(nodeRoutesApi); + await nodeRepository.getNodeHealth().toPromise(); expect(true).to.be.false; } catch (e) { - expect(e.message).contains('{"statusCode":404,"statusMessage":"Not Found","body":"'); + expect(e.message).eq('{"statusCode":666,"statusMessage":"Some status text error","body":"This is the body"}'); } });