From 75f637bd756fd7d4480792ead7bd86dd8326e622 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 19 Dec 2017 16:22:57 +0100 Subject: Throw a better error message when taker is null|undefined or anything but not a string --- packages/0x.js/src/0x.ts | 2 + .../src/contract_wrappers/exchange_wrapper.ts | 14 ++-- packages/0x.js/src/types.ts | 1 + packages/0x.js/src/utils/constants.ts | 1 + packages/0x.js/src/utils/decorators.ts | 84 +++++++++++++++++----- packages/0x.js/test/0x.js_test.ts | 9 +++ 6 files changed, 87 insertions(+), 24 deletions(-) (limited to 'packages') diff --git a/packages/0x.js/src/0x.ts b/packages/0x.js/src/0x.ts index 41fefb993..e4965f9a2 100644 --- a/packages/0x.js/src/0x.ts +++ b/packages/0x.js/src/0x.ts @@ -25,6 +25,7 @@ import { import {AbiDecoder} from './utils/abi_decoder'; import {assert} from './utils/assert'; import {constants} from './utils/constants'; +import {decorators} from './utils/decorators'; import {signatureUtils} from './utils/signature_utils'; import {utils} from './utils/utils'; @@ -155,6 +156,7 @@ export class ZeroEx { * @param order An object that conforms to the Order or SignedOrder interface definitions. * @return The resulting orderHash from hashing the supplied order. */ + @decorators.syncZeroExErrorHandler public static getOrderHashHex(order: Order|SignedOrder): string { assert.doesConformToSchema('order', order, schemas.orderSchema); const orderHashHex = utils.getOrderHashHex(order); diff --git a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts index df793aa06..70d2be7e9 100644 --- a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts @@ -162,7 +162,7 @@ export class ExchangeWrapper extends ContractWrapper { * @param orderTransactionOpts Optional arguments this method accepts. * @return Transaction hash. */ - @decorators.contractCallErrorHandler + @decorators.asyncZeroExErrorHandler public async fillOrderAsync(signedOrder: SignedOrder, fillTakerTokenAmount: BigNumber, shouldThrowOnInsufficientBalanceOrAllowance: boolean, takerAddress: string, @@ -218,7 +218,7 @@ export class ExchangeWrapper extends ContractWrapper { * @param orderTransactionOpts Optional arguments this method accepts. * @return Transaction hash. */ - @decorators.contractCallErrorHandler + @decorators.asyncZeroExErrorHandler public async fillOrdersUpToAsync(signedOrders: SignedOrder[], fillTakerTokenAmount: BigNumber, shouldThrowOnInsufficientBalanceOrAllowance: boolean, takerAddress: string, @@ -299,7 +299,7 @@ export class ExchangeWrapper extends ContractWrapper { * @param orderTransactionOpts Optional arguments this method accepts. * @return Transaction hash. */ - @decorators.contractCallErrorHandler + @decorators.asyncZeroExErrorHandler public async batchFillOrdersAsync(orderFillRequests: OrderFillRequest[], shouldThrowOnInsufficientBalanceOrAllowance: boolean, takerAddress: string, @@ -372,7 +372,7 @@ export class ExchangeWrapper extends ContractWrapper { * @param orderTransactionOpts Optional arguments this method accepts. * @return Transaction hash. */ - @decorators.contractCallErrorHandler + @decorators.asyncZeroExErrorHandler public async fillOrKillOrderAsync(signedOrder: SignedOrder, fillTakerTokenAmount: BigNumber, takerAddress: string, orderTransactionOpts: OrderTransactionOpts = {}): Promise { @@ -417,7 +417,7 @@ export class ExchangeWrapper extends ContractWrapper { * @param orderTransactionOpts Optional arguments this method accepts. * @return Transaction hash. */ - @decorators.contractCallErrorHandler + @decorators.asyncZeroExErrorHandler public async batchFillOrKillAsync(orderFillRequests: OrderFillRequest[], takerAddress: string, orderTransactionOpts: OrderTransactionOpts = {}): Promise { @@ -485,7 +485,7 @@ export class ExchangeWrapper extends ContractWrapper { * @param transactionOpts Optional arguments this method accepts. * @return Transaction hash. */ - @decorators.contractCallErrorHandler + @decorators.asyncZeroExErrorHandler public async cancelOrderAsync(order: Order|SignedOrder, cancelTakerTokenAmount: BigNumber, orderTransactionOpts: OrderTransactionOpts = {}): Promise { @@ -526,7 +526,7 @@ export class ExchangeWrapper extends ContractWrapper { * @param transactionOpts Optional arguments this method accepts. * @return Transaction hash. */ - @decorators.contractCallErrorHandler + @decorators.asyncZeroExErrorHandler public async batchCancelOrdersAsync(orderCancellationRequests: OrderCancellationRequest[], orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('orderCancellationRequests', orderCancellationRequests, diff --git a/packages/0x.js/src/types.ts b/packages/0x.js/src/types.ts index 844ac9ed9..704e59ce5 100644 --- a/packages/0x.js/src/types.ts +++ b/packages/0x.js/src/types.ts @@ -235,6 +235,7 @@ export interface OrderFillRequest { } export type AsyncMethod = (...args: any[]) => Promise; +export type SyncMethod = (...args: any[]) => any; /** * We re-export the `Web3.Provider` type specified in the Web3 Typescript typings diff --git a/packages/0x.js/src/utils/constants.ts b/packages/0x.js/src/utils/constants.ts index 3de3f5bc1..6001e1c7f 100644 --- a/packages/0x.js/src/utils/constants.ts +++ b/packages/0x.js/src/utils/constants.ts @@ -6,6 +6,7 @@ export const constants = { MAX_DIGITS_IN_UNSIGNED_256_INT: 78, INVALID_JUMP_PATTERN: 'invalid JUMP at', OUT_OF_GAS_PATTERN: 'out of gas', + INVALID_TAKER_FORMAT: 'instance.taker is not of a type(s) string', UNLIMITED_ALLOWANCE_IN_BASE_UNITS: new BigNumber(2).pow(256).minus(1), DEFAULT_BLOCK_POLLING_INTERVAL: 1000, }; diff --git a/packages/0x.js/src/utils/decorators.ts b/packages/0x.js/src/utils/decorators.ts index 1760d8872..060a26df9 100644 --- a/packages/0x.js/src/utils/decorators.ts +++ b/packages/0x.js/src/utils/decorators.ts @@ -1,17 +1,37 @@ import * as _ from 'lodash'; -import {AsyncMethod, ZeroExError} from '../types'; +import {AsyncMethod, SyncMethod, ZeroExError} from '../types'; import {constants} from './constants'; -export const decorators = { - /** - * Source: https://stackoverflow.com/a/29837695/3546986 - */ - contractCallErrorHandler(target: object, - key: string|symbol, - descriptor: TypedPropertyDescriptor, - ): TypedPropertyDescriptor { +type ErrorTransformer = (err: Error) => Error; + +const contractCallErrorTransformer = (error: Error) => { + if (_.includes(error.message, constants.INVALID_JUMP_PATTERN)) { + return new Error(ZeroExError.InvalidJump); + } + if (_.includes(error.message, constants.OUT_OF_GAS_PATTERN)) { + return new Error(ZeroExError.OutOfGas); + } + return error; +}; + +const schemaErrorTransformer = (error: Error) => { + if (_.includes(error.message, constants.INVALID_TAKER_FORMAT)) { + // tslint:disable-next-line:max-line-length + const errMsg = 'Order taker must be of type string. If you want anyone to be able to fill an order - pass ZeroEx.NULL_ADDRESS'; + throw new Error(errMsg); + } + return error; +}; + +/** + * Source: https://stackoverflow.com/a/29837695/3546986 + */ +const asyncErrorHandlerFactory = (errorTransformer: ErrorTransformer) => { + const asyncErrorHandlingDecorator = ( + target: object, key: string|symbol, descriptor: TypedPropertyDescriptor, + ) => { const originalMethod = (descriptor.value as AsyncMethod); // Do not use arrow syntax here. Use a function expression in @@ -22,16 +42,46 @@ export const decorators = { const result = await originalMethod.apply(this, args); return result; } catch (error) { - if (_.includes(error.message, constants.INVALID_JUMP_PATTERN)) { - throw new Error(ZeroExError.InvalidJump); - } - if (_.includes(error.message, constants.OUT_OF_GAS_PATTERN)) { - throw new Error(ZeroExError.OutOfGas); - } - throw error; + const transformedError = errorTransformer(error); + throw transformedError; } }; return descriptor; - }, + }; + + return asyncErrorHandlingDecorator; +}; + +const syncErrorHandlerFactory = (errorTransformer: ErrorTransformer) => { + const syncErrorHandlingDecorator = ( + target: object, key: string|symbol, descriptor: TypedPropertyDescriptor, + ) => { + const originalMethod = (descriptor.value as SyncMethod); + + // Do not use arrow syntax here. Use a function expression in + // order to use the correct value of `this` in this method + // tslint:disable-next-line:only-arrow-functions + descriptor.value = function(...args: any[]) { + try { + const result = originalMethod.apply(this, args); + return result; + } catch (error) { + const transformedError = errorTransformer(error); + throw transformedError; + } + }; + + return descriptor; + }; + + return syncErrorHandlingDecorator; +}; + +// _.flow(f, g) = f ∘ g +const zeroExErrorTransformer = _.flow(schemaErrorTransformer, contractCallErrorTransformer); + +export const decorators = { + asyncZeroExErrorHandler: asyncErrorHandlerFactory(zeroExErrorTransformer), + syncZeroExErrorHandler: syncErrorHandlerFactory(zeroExErrorTransformer), }; diff --git a/packages/0x.js/test/0x.js_test.ts b/packages/0x.js/test/0x.js_test.ts index 819ac12f9..8d62b3518 100644 --- a/packages/0x.js/test/0x.js_test.ts +++ b/packages/0x.js/test/0x.js_test.ts @@ -152,6 +152,15 @@ describe('ZeroEx library', () => { const orderHash = ZeroEx.getOrderHashHex(order); expect(orderHash).to.be.equal(expectedOrderHash); }); + it('throws a readable error message if taker format is invalid', async () => { + const orderWithInvalidtakerFormat = { + ...order, + taker: null as any as string, + }; + // tslint:disable-next-line:max-line-length + const expectedErrorMessage = 'Order taker must be of type string. If you want anyone to be able to fill an order - pass ZeroEx.NULL_ADDRESS'; + expect(() => ZeroEx.getOrderHashHex(orderWithInvalidtakerFormat)).to.throw(expectedErrorMessage); + }); }); describe('#signOrderHashAsync', () => { let stubs: Sinon.SinonStub[] = []; -- cgit v1.2.3 From 93518802d6d5baf76c8c66e825eddd4241d4fa7c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 19 Dec 2017 16:26:05 +0100 Subject: Update CHANGELOG --- packages/0x.js/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) (limited to 'packages') diff --git a/packages/0x.js/CHANGELOG.md b/packages/0x.js/CHANGELOG.md index 962d312d3..c69edd88c 100644 --- a/packages/0x.js/CHANGELOG.md +++ b/packages/0x.js/CHANGELOG.md @@ -6,6 +6,7 @@ v0.x.x - _TBD, 2017_ * Removed accidentally included `unsubscribeAll` method from `zeroEx.proxy`, `zeroEx.etherToken` and `zeroEx.tokenRegistry` (#267) * Removed `etherTokenContractAddress` from `ZeroEx` constructor arg `ZeroExConfig` (#267) * Rename `SubscriptionOpts` to `BlockRange` (#272) + * Improve the error message when taker is not a string (#278) v0.27.1 - _November 28, 2017_ ------------------------ -- cgit v1.2.3 From 3c66f18a46ae297181bdc82023a8e8bd895eebe1 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 19 Dec 2017 18:39:35 +0100 Subject: Don't throw in transformers --- packages/0x.js/src/utils/decorators.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages') diff --git a/packages/0x.js/src/utils/decorators.ts b/packages/0x.js/src/utils/decorators.ts index 060a26df9..2a823b9ac 100644 --- a/packages/0x.js/src/utils/decorators.ts +++ b/packages/0x.js/src/utils/decorators.ts @@ -20,7 +20,7 @@ const schemaErrorTransformer = (error: Error) => { if (_.includes(error.message, constants.INVALID_TAKER_FORMAT)) { // tslint:disable-next-line:max-line-length const errMsg = 'Order taker must be of type string. If you want anyone to be able to fill an order - pass ZeroEx.NULL_ADDRESS'; - throw new Error(errMsg); + return new Error(errMsg); } return error; }; -- cgit v1.2.3