From 4262ac3c8966468b343c6467e0e9da85ef25be05 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 10 Nov 2017 14:13:15 -0500 Subject: Fix unhandled promise rejection error on subscriptions --- src/contract_wrappers/contract_wrapper.ts | 25 ++++++++++++++++++------- src/types.ts | 4 ++-- 2 files changed, 20 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/contract_wrappers/contract_wrapper.ts b/src/contract_wrappers/contract_wrapper.ts index 19dccc6f2..a02465982 100644 --- a/src/contract_wrappers/contract_wrapper.ts +++ b/src/contract_wrappers/contract_wrapper.ts @@ -50,10 +50,14 @@ export class ContractWrapper { this._filterCallbacks[filterToken] = callback; return filterToken; } - protected _unsubscribe(filterToken: string): void { + protected _unsubscribe(filterToken: string, err?: Error): void { if (_.isUndefined(this._filters[filterToken])) { throw new Error(ZeroExError.SubscriptionNotFound); } + if (!_.isUndefined(err)) { + const callback = this._filterCallbacks[filterToken]; + callback(err, undefined); + } delete this._filters[filterToken]; delete this._filterCallbacks[filterToken]; if (_.isEmpty(this._filters)) { @@ -90,7 +94,7 @@ export class ContractWrapper { ...decodedLog, removed, }; - this._filterCallbacks[filterToken](logEvent); + this._filterCallbacks[filterToken](null, logEvent); } }); } @@ -122,11 +126,18 @@ export class ContractWrapper { delete this._blockAndLogStreamer; } private async _reconcileBlockAsync(): Promise { - const latestBlock = await this._web3Wrapper.getBlockAsync(BlockParamLiteral.Latest); - // We need to coerce to Block type cause Web3.Block includes types for mempool blocks - if (!_.isUndefined(this._blockAndLogStreamer)) { - // If we clear the interval while fetching the block - this._blockAndLogStreamer will be undefined - this._blockAndLogStreamer.reconcileNewBlock(latestBlock as any as Block); + try { + const latestBlock = await this._web3Wrapper.getBlockAsync(BlockParamLiteral.Latest); + // We need to coerce to Block type cause Web3.Block includes types for mempool blocks + if (!_.isUndefined(this._blockAndLogStreamer)) { + // If we clear the interval while fetching the block - this._blockAndLogStreamer will be undefined + this._blockAndLogStreamer.reconcileNewBlock(latestBlock as any as Block); + } + } catch (err) { + const filterTokens = _.keys(this._filterCallbacks); + _.each(filterTokens, filterToken => { + this._unsubscribe(filterToken, err); + }); } } } diff --git a/src/types.ts b/src/types.ts index a9eac56d8..af3c36736 100644 --- a/src/types.ts +++ b/src/types.ts @@ -41,8 +41,8 @@ export type OrderValues = [BigNumber, BigNumber, BigNumber, export interface LogEvent extends LogWithDecodedArgs { removed: boolean; } -export type EventCallbackAsync = (log: LogEvent) => Promise; -export type EventCallbackSync = (log: LogEvent) => void; +export type EventCallbackAsync = (err: null|Error, log?: LogEvent) => Promise; +export type EventCallbackSync = (err: null|Error, log?: LogEvent) => void; export type EventCallback = EventCallbackSync|EventCallbackAsync; export interface ExchangeContract extends Web3.ContractInstance { isValidSignature: { -- cgit v1.2.3 From 4ae9482d506ccdf028fcd39bbaf652bbcbef52e8 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Sat, 11 Nov 2017 11:01:59 -0500 Subject: Clean up subscription state. In the case of an exception, keep the state correct between contract wrapper, exchange wrapper and token wrapper. --- src/contract_wrappers/contract_wrapper.ts | 6 ++++++ src/contract_wrappers/exchange_wrapper.ts | 18 +++++++----------- src/contract_wrappers/token_wrapper.ts | 7 +------ 3 files changed, 14 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/contract_wrappers/contract_wrapper.ts b/src/contract_wrappers/contract_wrapper.ts index a02465982..5bdc66b3b 100644 --- a/src/contract_wrappers/contract_wrapper.ts +++ b/src/contract_wrappers/contract_wrapper.ts @@ -50,6 +50,12 @@ export class ContractWrapper { this._filterCallbacks[filterToken] = callback; return filterToken; } + protected unsubscribeAll(): void { + const filterTokens = _.keys(this._filterCallbacks); + _.each(filterTokens, filterToken => { + this._unsubscribe(filterToken); + }); + } protected _unsubscribe(filterToken: string, err?: Error): void { if (_.isUndefined(this._filters[filterToken])) { throw new Error(ZeroExError.SubscriptionNotFound); diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index b8704e72c..4fd2b23a8 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -49,7 +49,6 @@ const SHOULD_VALIDATE_BY_DEFAULT = true; */ export class ExchangeWrapper extends ContractWrapper { private _exchangeContractIfExists?: ExchangeContract; - private _activeSubscriptions: string[]; private _orderValidationUtils: OrderValidationUtils; private _tokenWrapper: TokenWrapper; private _exchangeContractErrCodesToMsg = { @@ -84,7 +83,6 @@ export class ExchangeWrapper extends ContractWrapper { super(web3Wrapper, abiDecoder); this._tokenWrapper = tokenWrapper; this._orderValidationUtils = new OrderValidationUtils(tokenWrapper, this); - this._activeSubscriptions = []; this._contractAddressIfExists = contractAddressIfExists; } /** @@ -666,7 +664,6 @@ export class ExchangeWrapper extends ContractWrapper { const subscriptionToken = this._subscribe( exchangeContractAddress, eventName, indexFilterValues, artifacts.ExchangeArtifact.abi, callback, ); - this._activeSubscriptions.push(subscriptionToken); return subscriptionToken; } /** @@ -674,9 +671,14 @@ export class ExchangeWrapper extends ContractWrapper { * @param subscriptionToken Subscription token returned by `subscribe()` */ public unsubscribe(subscriptionToken: string): void { - _.pull(this._activeSubscriptions, subscriptionToken); this._unsubscribe(subscriptionToken); } + /** + * Cancels all existing subscriptions + */ + public unsubscribeAll(): void { + super.unsubscribeAll(); + } /** * Gets historical logs without creating a subscription * @param eventName The exchange contract event you would like to subscribe to. @@ -825,13 +827,7 @@ export class ExchangeWrapper extends ContractWrapper { const ZRXtokenAddress = await exchangeInstance.ZRX_TOKEN_CONTRACT.callAsync(); return ZRXtokenAddress; } - /** - * Cancels all existing subscriptions - */ - public unsubscribeAll(): void { - _.forEach(this._activeSubscriptions, this._unsubscribe.bind(this)); - this._activeSubscriptions = []; - } + private async _invalidateContractInstancesAsync(): Promise { this.unsubscribeAll(); delete this._exchangeContractIfExists; diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index 5d6d61cef..fbe7354dc 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -29,13 +29,11 @@ const ALLOWANCE_TO_ZERO_GAS_AMOUNT = 47275; export class TokenWrapper extends ContractWrapper { public UNLIMITED_ALLOWANCE_IN_BASE_UNITS = constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS; private _tokenContractsByAddress: {[address: string]: TokenContract}; - private _activeSubscriptions: string[]; private _tokenTransferProxyContractAddressFetcher: () => Promise; constructor(web3Wrapper: Web3Wrapper, abiDecoder: AbiDecoder, tokenTransferProxyContractAddressFetcher: () => Promise) { super(web3Wrapper, abiDecoder); this._tokenContractsByAddress = {}; - this._activeSubscriptions = []; this._tokenTransferProxyContractAddressFetcher = tokenTransferProxyContractAddressFetcher; } /** @@ -262,7 +260,6 @@ export class TokenWrapper extends ContractWrapper { const subscriptionToken = this._subscribe( tokenAddress, eventName, indexFilterValues, artifacts.TokenArtifact.abi, callback, ); - this._activeSubscriptions.push(subscriptionToken); return subscriptionToken; } /** @@ -270,7 +267,6 @@ export class TokenWrapper extends ContractWrapper { * @param subscriptionToken Subscription token returned by `subscribe()` */ public unsubscribe(subscriptionToken: string): void { - _.pull(this._activeSubscriptions, subscriptionToken); this._unsubscribe(subscriptionToken); } /** @@ -298,8 +294,7 @@ export class TokenWrapper extends ContractWrapper { * Cancels all existing subscriptions */ public unsubscribeAll(): void { - _.forEach(this._activeSubscriptions, this._unsubscribe.bind(this)); - this._activeSubscriptions = []; + super.unsubscribeAll(); } private _invalidateContractInstancesAsync(): void { this.unsubscribeAll(); -- cgit v1.2.3 From 394417ff073c54ba7409ac24004a9757935688cb Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Sat, 11 Nov 2017 12:15:24 -0500 Subject: Removed nits --- src/contract_wrappers/exchange_wrapper.ts | 1 - 1 file changed, 1 deletion(-) (limited to 'src') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 4fd2b23a8..711d883ef 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -827,7 +827,6 @@ export class ExchangeWrapper extends ContractWrapper { const ZRXtokenAddress = await exchangeInstance.ZRX_TOKEN_CONTRACT.callAsync(); return ZRXtokenAddress; } - private async _invalidateContractInstancesAsync(): Promise { this.unsubscribeAll(); delete this._exchangeContractIfExists; -- cgit v1.2.3 From d34ea79d939beaf677c6730fe6e84e25a687d59f Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Sat, 11 Nov 2017 13:42:42 -0500 Subject: Push unsubscribe to the base class rather than super --- src/contract_wrappers/contract_wrapper.ts | 29 ++++++++++++++++------------- src/contract_wrappers/exchange_wrapper.ts | 6 ------ src/contract_wrappers/token_wrapper.ts | 6 ------ 3 files changed, 16 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/contract_wrappers/contract_wrapper.ts b/src/contract_wrappers/contract_wrapper.ts index 5bdc66b3b..7997b1647 100644 --- a/src/contract_wrappers/contract_wrapper.ts +++ b/src/contract_wrappers/contract_wrapper.ts @@ -38,19 +38,10 @@ export class ContractWrapper { this._onLogAddedSubscriptionToken = undefined; this._onLogRemovedSubscriptionToken = undefined; } - protected _subscribe( - address: string, eventName: ContractEvents, indexFilterValues: IndexedFilterValues, abi: Web3.ContractAbi, - callback: EventCallback): string { - const filter = filterUtils.getFilter(address, eventName, indexFilterValues, abi); - if (_.isUndefined(this._blockAndLogStreamer)) { - this._startBlockAndLogStream(); - } - const filterToken = filterUtils.generateUUID(); - this._filters[filterToken] = filter; - this._filterCallbacks[filterToken] = callback; - return filterToken; - } - protected unsubscribeAll(): void { + /** + * Cancels all existing subscriptions + */ + public unsubscribeAll(): void { const filterTokens = _.keys(this._filterCallbacks); _.each(filterTokens, filterToken => { this._unsubscribe(filterToken); @@ -70,6 +61,18 @@ export class ContractWrapper { this._stopBlockAndLogStream(); } } + protected _subscribe( + address: string, eventName: ContractEvents, indexFilterValues: IndexedFilterValues, abi: Web3.ContractAbi, + callback: EventCallback): string { + const filter = filterUtils.getFilter(address, eventName, indexFilterValues, abi); + if (_.isUndefined(this._blockAndLogStreamer)) { + this._startBlockAndLogStream(); + } + const filterToken = filterUtils.generateUUID(); + this._filters[filterToken] = filter; + this._filterCallbacks[filterToken] = callback; + return filterToken; + } protected async _getLogsAsync( address: string, eventName: ContractEvents, subscriptionOpts: SubscriptionOpts, indexFilterValues: IndexedFilterValues, abi: Web3.ContractAbi): Promise>> { diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 711d883ef..b608e6931 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -673,12 +673,6 @@ export class ExchangeWrapper extends ContractWrapper { public unsubscribe(subscriptionToken: string): void { this._unsubscribe(subscriptionToken); } - /** - * Cancels all existing subscriptions - */ - public unsubscribeAll(): void { - super.unsubscribeAll(); - } /** * Gets historical logs without creating a subscription * @param eventName The exchange contract event you would like to subscribe to. diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index fbe7354dc..081113964 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -290,12 +290,6 @@ export class TokenWrapper extends ContractWrapper { ); return logs; } - /** - * Cancels all existing subscriptions - */ - public unsubscribeAll(): void { - super.unsubscribeAll(); - } private _invalidateContractInstancesAsync(): void { this.unsubscribeAll(); this._tokenContractsByAddress = {}; -- cgit v1.2.3