Merge branch 'development' into orderWatcher

* development:
  0.23.0
  Update CHANGELOG
  Fix amounts in tests one last time. Now that we updated the testRPC snapshot, this should no longer be mismatched between CI and locally
  Update testRPC snapshot used by CircleCi
  Push unsubscribe to the base class rather than super
  Check for null rather than undefined
  Removed nits
  Test case was error then unsubscribe
  Clean up subscription state.
  Fix unhandled promise rejection error on subscriptions

# Conflicts:
#	src/types.ts
#	test/exchange_wrapper_test.ts
#	test/token_wrapper_test.ts

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(