diff options
-rw-r--r-- | CHANGELOG.md | 5 | ||||
-rw-r--r-- | package-lock.json | 2 | ||||
-rw-r--r-- | package.json | 2 | ||||
-rw-r--r-- | src/contract_wrappers/contract_wrapper.ts | 52 | ||||
-rw-r--r-- | src/contract_wrappers/exchange_wrapper.ts | 11 | ||||
-rw-r--r-- | src/contract_wrappers/token_wrapper.ts | 11 | ||||
-rw-r--r-- | src/types.ts | 4 | ||||
-rw-r--r-- | test/exchange_wrapper_test.ts | 13 | ||||
-rw-r--r-- | test/subscription_test.ts | 95 | ||||
-rw-r--r-- | test/token_wrapper_test.ts | 10 |
10 files changed, 153 insertions, 52 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 029144b5a..00164bb74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # CHANGELOG +v0.23.0 - _November 12, 2017_ +------------------------ + * Fixed unhandled promise rejection error in subscribe methods (#209) + * Subscribe callbacks now receive an error object as their first argument + v0.22.6 - _November 10, 2017_ ------------------------ * Add a timeout parameter to transaction awaiting (#206) diff --git a/package-lock.json b/package-lock.json index 6b4a97d87..8e2823103 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "0x.js", - "version": "0.22.6", + "version": "0.23.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index e7e21bdce..171cc7706 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "0x.js", - "version": "0.22.6", + "version": "0.23.0", "description": "A javascript library for interacting with the 0x protocol", "keywords": [ "0x.js", diff --git a/src/contract_wrappers/contract_wrapper.ts b/src/contract_wrappers/contract_wrapper.ts index 19dccc6f2..7997b1647 100644 --- a/src/contract_wrappers/contract_wrapper.ts +++ b/src/contract_wrappers/contract_wrapper.ts @@ -38,6 +38,29 @@ export class ContractWrapper { this._onLogAddedSubscriptionToken = undefined; this._onLogRemovedSubscriptionToken = undefined; } + /** + * Cancels all existing subscriptions + */ + public 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); + } + if (!_.isUndefined(err)) { + const callback = this._filterCallbacks[filterToken]; + callback(err, undefined); + } + delete this._filters[filterToken]; + delete this._filterCallbacks[filterToken]; + if (_.isEmpty(this._filters)) { + this._stopBlockAndLogStream(); + } + } protected _subscribe<ArgsType extends ContractEventArgs>( address: string, eventName: ContractEvents, indexFilterValues: IndexedFilterValues, abi: Web3.ContractAbi, callback: EventCallback<ArgsType>): string { @@ -50,16 +73,6 @@ export class ContractWrapper { this._filterCallbacks[filterToken] = callback; return filterToken; } - protected _unsubscribe(filterToken: string): void { - if (_.isUndefined(this._filters[filterToken])) { - throw new Error(ZeroExError.SubscriptionNotFound); - } - delete this._filters[filterToken]; - delete this._filterCallbacks[filterToken]; - if (_.isEmpty(this._filters)) { - this._stopBlockAndLogStream(); - } - } protected async _getLogsAsync<ArgsType extends ContractEventArgs>( address: string, eventName: ContractEvents, subscriptionOpts: SubscriptionOpts, indexFilterValues: IndexedFilterValues, abi: Web3.ContractAbi): Promise<Array<LogWithDecodedArgs<ArgsType>>> { @@ -90,7 +103,7 @@ export class ContractWrapper { ...decodedLog, removed, }; - this._filterCallbacks[filterToken](logEvent); + this._filterCallbacks[filterToken](null, logEvent); } }); } @@ -122,11 +135,18 @@ export class ContractWrapper { delete this._blockAndLogStreamer; } private async _reconcileBlockAsync(): Promise<void> { - 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/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index f68e80b5d..fe0c5bc00 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -48,7 +48,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 = { @@ -83,7 +82,6 @@ export class ExchangeWrapper extends ContractWrapper { super(web3Wrapper, abiDecoder); this._tokenWrapper = tokenWrapper; this._orderValidationUtils = new OrderValidationUtils(tokenWrapper, this); - this._activeSubscriptions = []; this._contractAddressIfExists = contractAddressIfExists; } /** @@ -665,7 +663,6 @@ export class ExchangeWrapper extends ContractWrapper { const subscriptionToken = this._subscribe<ArgsType>( exchangeContractAddress, eventName, indexFilterValues, artifacts.ExchangeArtifact.abi, callback, ); - this._activeSubscriptions.push(subscriptionToken); return subscriptionToken; } /** @@ -673,7 +670,6 @@ export class ExchangeWrapper extends ContractWrapper { * @param subscriptionToken Subscription token returned by `subscribe()` */ public unsubscribe(subscriptionToken: string): void { - _.pull(this._activeSubscriptions, subscriptionToken); this._unsubscribe(subscriptionToken); } /** @@ -824,13 +820,6 @@ 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<void> { this.unsubscribeAll(); delete this._exchangeContractIfExists; diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index dd87ebc9f..614ac19d4 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<string>; constructor(web3Wrapper: Web3Wrapper, abiDecoder: AbiDecoder, tokenTransferProxyContractAddressFetcher: () => Promise<string>) { super(web3Wrapper, abiDecoder); this._tokenContractsByAddress = {}; - this._activeSubscriptions = []; this._tokenTransferProxyContractAddressFetcher = tokenTransferProxyContractAddressFetcher; } /** @@ -262,7 +260,6 @@ export class TokenWrapper extends ContractWrapper { const subscriptionToken = this._subscribe<ArgsType>( 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); } /** @@ -294,13 +290,6 @@ export class TokenWrapper extends ContractWrapper { ); return logs; } - /** - * Cancels all existing subscriptions - */ - public unsubscribeAll(): void { - _.forEach(this._activeSubscriptions, this._unsubscribe.bind(this)); - this._activeSubscriptions = []; - } private _invalidateContractInstancesAsync(): void { this.unsubscribeAll(); this._tokenContractsByAddress = {}; diff --git a/src/types.ts b/src/types.ts index a366fc31e..09b611019 100644 --- a/src/types.ts +++ b/src/types.ts @@ -42,8 +42,8 @@ export type OrderValues = [BigNumber, BigNumber, BigNumber, export type LogEvent = Web3.LogEntryEvent; export type DecodedLogEvent<ArgsType> = Web3.DecodedLogEntryEvent<ArgsType>; -export type EventCallbackAsync<ArgsType> = (log: DecodedLogEvent<ArgsType>) => Promise<void>; -export type EventCallbackSync<ArgsType> = (log: DecodedLogEvent<ArgsType>) => void; +export type EventCallbackAsync<ArgsType> = (err: null|Error, log?: DecodedLogEvent<ArgsType>) => Promise<void>; +export type EventCallbackSync<ArgsType> = (err: null|Error, log?: DecodedLogEvent<ArgsType>) => void; export type EventCallback<ArgsType> = EventCallbackSync<ArgsType>|EventCallbackAsync<ArgsType>; export type EventWatcherCallbackSync = (log: LogEvent) => void; diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 20b9cf7fc..26b8c1e0e 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -647,7 +647,8 @@ describe('ExchangeWrapper', () => { // Source: https://github.com/mochajs/mocha/issues/2407 it('Should receive the LogFill event when an order is filled', (done: DoneCallback) => { (async () => { - const callback = (logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { + + const callback = (err: Error, logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { expect(logEvent.event).to.be.equal(ExchangeEvents.LogFill); done(); }; @@ -662,7 +663,8 @@ describe('ExchangeWrapper', () => { }); it('Should receive the LogCancel event when an order is cancelled', (done: DoneCallback) => { (async () => { - const callback = (logEvent: DecodedLogEvent<LogCancelContractEventArgs>) => { + + const callback = (err: Error, logEvent: DecodedLogEvent<LogCancelContractEventArgs>) => { expect(logEvent.event).to.be.equal(ExchangeEvents.LogCancel); done(); }; @@ -674,7 +676,8 @@ describe('ExchangeWrapper', () => { }); it('Outstanding subscriptions are cancelled when zeroEx.setProviderAsync called', (done: DoneCallback) => { (async () => { - const callbackNeverToBeCalled = (logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { + + const callbackNeverToBeCalled = (err: Error, logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { done(new Error('Expected this subscription to have been cancelled')); }; await zeroEx.exchange.subscribeAsync( @@ -684,7 +687,7 @@ describe('ExchangeWrapper', () => { const newProvider = web3Factory.getRpcProvider(); await zeroEx.setProviderAsync(newProvider); - const callback = (logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { + const callback = (err: Error, logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { expect(logEvent.event).to.be.equal(ExchangeEvents.LogFill); done(); }; @@ -699,7 +702,7 @@ describe('ExchangeWrapper', () => { }); it('Should cancel subscription when unsubscribe called', (done: DoneCallback) => { (async () => { - const callbackNeverToBeCalled = (logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { + const callbackNeverToBeCalled = (err: Error, logEvent: DecodedLogEvent<LogFillContractEventArgs>) => { done(new Error('Expected this subscription to have been cancelled')); }; const subscriptionToken = await zeroEx.exchange.subscribeAsync( diff --git a/test/subscription_test.ts b/test/subscription_test.ts new file mode 100644 index 000000000..985fdc1d6 --- /dev/null +++ b/test/subscription_test.ts @@ -0,0 +1,95 @@ +import 'mocha'; +import * as _ from 'lodash'; +import * as chai from 'chai'; +import * as Sinon from 'sinon'; +import {chaiSetup} from './utils/chai_setup'; +import * as Web3 from 'web3'; +import BigNumber from 'bignumber.js'; +import promisify = require('es6-promisify'); +import {web3Factory} from './utils/web3_factory'; +import { + ZeroEx, + ZeroExError, + Token, + ApprovalContractEventArgs, + TokenEvents, + LogEvent, +} from '../src'; +import {BlockchainLifecycle} from './utils/blockchain_lifecycle'; +import {TokenUtils} from './utils/token_utils'; +import {DoneCallback, BlockParamLiteral} from '../src/types'; + +chaiSetup.configure(); +const expect = chai.expect; +const blockchainLifecycle = new BlockchainLifecycle(); + +describe('SubscriptionTest', () => { + let web3: Web3; + let zeroEx: ZeroEx; + let userAddresses: string[]; + let tokens: Token[]; + let tokenUtils: TokenUtils; + let coinbase: string; + let addressWithoutFunds: string; + before(async () => { + web3 = web3Factory.create(); + zeroEx = new ZeroEx(web3.currentProvider); + userAddresses = await zeroEx.getAvailableAddressesAsync(); + tokens = await zeroEx.tokenRegistry.getTokensAsync(); + tokenUtils = new TokenUtils(tokens); + coinbase = userAddresses[0]; + addressWithoutFunds = userAddresses[1]; + }); + beforeEach(async () => { + await blockchainLifecycle.startAsync(); + }); + afterEach(async () => { + await blockchainLifecycle.revertAsync(); + }); + describe('#subscribe', () => { + const indexFilterValues = {}; + const shouldThrowOnInsufficientBalanceOrAllowance = true; + let tokenAddress: string; + const transferAmount = new BigNumber(42); + const allowanceAmount = new BigNumber(42); + let stubs: Sinon.SinonStub[] = []; + before(() => { + const token = tokens[0]; + tokenAddress = token.address; + }); + afterEach(() => { + zeroEx.token.unsubscribeAll(); + _.each(stubs, s => s.restore()); + stubs = []; + }); + it('Should receive the Error when an error occurs', (done: DoneCallback) => { + (async () => { + const callback = (err: Error, logEvent: LogEvent<ApprovalContractEventArgs>) => { + expect(err).to.not.be.null(); + expect(logEvent).to.be.undefined(); + done(); + }; + stubs = [ + Sinon.stub((zeroEx as any)._web3Wrapper, 'getBlockAsync') + .throws("JSON RPC error") + ] + zeroEx.token.subscribe( + tokenAddress, TokenEvents.Approval, indexFilterValues, callback); + await zeroEx.token.setAllowanceAsync(tokenAddress, coinbase, addressWithoutFunds, allowanceAmount); + })().catch(done); + }); + it('Should allow unsubscribeAll to be called successfully after an error', (done: DoneCallback) => { + (async () => { + const callback = (err: Error, logEvent: LogEvent<ApprovalContractEventArgs>) => { }; + zeroEx.token.subscribe( + tokenAddress, TokenEvents.Approval, indexFilterValues, callback); + stubs = [ + Sinon.stub((zeroEx as any)._web3Wrapper, 'getBlockAsync') + .throws("JSON RPC error") + ] + zeroEx.token.unsubscribeAll(); + done(); + })().catch(done); + }); + }) + })
\ No newline at end of file diff --git a/test/token_wrapper_test.ts b/test/token_wrapper_test.ts index 23020c47a..40652d03c 100644 --- a/test/token_wrapper_test.ts +++ b/test/token_wrapper_test.ts @@ -359,7 +359,7 @@ describe('TokenWrapper', () => { // Source: https://github.com/mochajs/mocha/issues/2407 it('Should receive the Transfer event when tokens are transfered', (done: DoneCallback) => { (async () => { - const callback = (logEvent: DecodedLogEvent<TransferContractEventArgs>) => { + const callback = (err: Error, logEvent: DecodedLogEvent<TransferContractEventArgs>) => { expect(logEvent).to.not.be.undefined(); const args = logEvent.args; expect(args._from).to.be.equal(coinbase); @@ -374,7 +374,7 @@ describe('TokenWrapper', () => { }); it('Should receive the Approval event when allowance is being set', (done: DoneCallback) => { (async () => { - const callback = (logEvent: DecodedLogEvent<ApprovalContractEventArgs>) => { + const callback = (err: Error, logEvent: DecodedLogEvent<ApprovalContractEventArgs>) => { expect(logEvent).to.not.be.undefined(); const args = logEvent.args; expect(args._owner).to.be.equal(coinbase); @@ -389,13 +389,13 @@ describe('TokenWrapper', () => { }); it('Outstanding subscriptions are cancelled when zeroEx.setProviderAsync called', (done: DoneCallback) => { (async () => { - const callbackNeverToBeCalled = (logEvent: DecodedLogEvent<TransferContractEventArgs>) => { + const callbackNeverToBeCalled = (err: Error, logEvent: DecodedLogEvent<ApprovalContractEventArgs>) => { done(new Error('Expected this subscription to have been cancelled')); }; zeroEx.token.subscribe( tokenAddress, TokenEvents.Transfer, indexFilterValues, callbackNeverToBeCalled, ); - const callbackToBeCalled = (logEvent: DecodedLogEvent<TransferContractEventArgs>) => { + const callbackToBeCalled = (err: Error, logEvent: DecodedLogEvent<ApprovalContractEventArgs>) => { done(); }; const newProvider = web3Factory.getRpcProvider(); @@ -408,7 +408,7 @@ describe('TokenWrapper', () => { }); it('Should cancel subscription when unsubscribe called', (done: DoneCallback) => { (async () => { - const callbackNeverToBeCalled = (logEvent: DecodedLogEvent<TokenContractEventArgs>) => { + const callbackNeverToBeCalled = (err: Error, logEvent: DecodedLogEvent<ApprovalContractEventArgs>) => { done(new Error('Expected this subscription to have been cancelled')); }; const subscriptionToken = zeroEx.token.subscribe( |