Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Commit 07af2e0

Browse files
nividanivida
authored
Merge pull request #2740 from ethereum/enhancement/error-handling-call-method
Error handling for callbacks fixed.
2 parents 54c6650 + 2b43929 commit 07af2e0

File tree

6 files changed

+119
-36
lines changed

6 files changed

+119
-36
lines changed

‎packages/web3-core-method/lib/methods/AbstractMethod.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,17 @@ export default class AbstractMethod {
7979
this.beforeExecution(this.moduleInstance);
8080

8181
if (this.parameters.length !== this.parametersAmount) {
82-
throw new Error(
82+
const error = new Error(
8383
`Invalid Arguments length: expected: ${this.parametersAmount}, given: ${this.parameters.length}`
8484
);
85+
86+
if (this.callback) {
87+
this.callback(error, null);
88+
89+
return;
90+
}
91+
92+
throw error;
8593
}
8694

8795
try {
@@ -93,12 +101,16 @@ export default class AbstractMethod {
93101

94102
if (this.callback) {
95103
this.callback(false, response);
104+
105+
return;
96106
}
97107

98108
return response;
99109
} catch (error) {
100110
if (this.callback) {
101111
this.callback(error, null);
112+
113+
return;
102114
}
103115

104116
throw error;

‎packages/web3-core-method/tests/lib/methods/AbstractMethodTest.js

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,17 @@ jest.mock('web3-providers');
1212
* AbstractMethod test
1313
*/
1414
describe('AbstractMethodTest', () => {
15-
let abstractMethod, moduleInstanceMock, providerMock, callback;
15+
let abstractMethod, moduleInstanceMock, providerMock;
1616

1717
beforeEach(() => {
18-
callback = jest.fn();
19-
2018
new WebsocketProvider('host', {});
2119
providerMock = WebsocketProvider.mock.instances[0];
2220
providerMock.send = jest.fn();
2321

2422
moduleInstanceMock = {};
2523

2624
abstractMethod = new AbstractMethod('RPC_TEST', 0, Utils, formatters, moduleInstanceMock);
27-
abstractMethod.callback = callback;
25+
abstractMethod.callback = false;
2826
abstractMethod.beforeExecution = jest.fn();
2927
});
3028

@@ -41,7 +39,7 @@ describe('AbstractMethodTest', () => {
4139

4240
expect(abstractMethod.parameters).toEqual([]);
4341

44-
expect(abstractMethod.callback).toEqual(callback);
42+
expect(abstractMethod.callback).toEqual(false);
4543
});
4644

4745
it('setArguments throws error on missing arguments', () => {
@@ -138,29 +136,47 @@ describe('AbstractMethodTest', () => {
138136

139137
expect(providerMock.send).toHaveBeenCalledWith(abstractMethod.rpcMethod, abstractMethod.parameters);
140138

141-
expect(abstractMethod.callback).toHaveBeenCalledWith(false, '0x00');
142-
143139
expect(abstractMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
144140

145141
expect(abstractMethod.afterExecution).toHaveBeenCalledWith('0x0');
146142
});
147143

148-
it('calls execute and throws an error on sending the request to the connected node', async () => {
144+
it('calls execute and returns a rejected promise on sending the request to the connected node', async () => {
149145
providerMock.send = jest.fn(() => {
150146
return Promise.reject(new Error('ERROR ON SEND'));
151147
});
152148

149+
abstractMethod.callback = false;
153150
moduleInstanceMock.currentProvider = providerMock;
154151
await expect(abstractMethod.execute(moduleInstanceMock)).rejects.toThrow('ERROR ON SEND');
155152

156153
expect(providerMock.send).toHaveBeenCalledWith(abstractMethod.rpcMethod, abstractMethod.parameters);
157154

158-
expect(abstractMethod.callback).toHaveBeenCalledWith(new Error('ERROR ON SEND'), null);
159-
160155
expect(abstractMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
161156
});
162157

163-
it('calls execute and throws an error because of a invalid parameters length', async () => {
158+
it('calls execute and throws an error on sending the request to the connected node', (done) => {
159+
providerMock.send = jest.fn(() => {
160+
return Promise.reject(new Error('ERROR ON SEND'));
161+
});
162+
163+
abstractMethod.callback = jest.fn((error, response) => {
164+
expect(error).toEqual(new Error('ERROR ON SEND'));
165+
166+
expect(response).toEqual(null);
167+
168+
expect(providerMock.send).toHaveBeenCalledWith(abstractMethod.rpcMethod, abstractMethod.parameters);
169+
170+
expect(abstractMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
171+
172+
done();
173+
});
174+
175+
moduleInstanceMock.currentProvider = providerMock;
176+
abstractMethod.execute(moduleInstanceMock);
177+
});
178+
179+
it('calls execute and returns a rejected promise because of a invalid parameters length', async () => {
164180
abstractMethod.parametersAmount = 0;
165181
abstractMethod.parameters = [true];
166182

@@ -171,6 +187,23 @@ describe('AbstractMethodTest', () => {
171187
expect(abstractMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
172188
});
173189

190+
it('calls execute and throws an error because of a invalid parameters length', (done) => {
191+
abstractMethod.parametersAmount = 0;
192+
abstractMethod.parameters = [true];
193+
194+
abstractMethod.callback = jest.fn((error, response) => {
195+
expect(error).toEqual(new Error('Invalid Arguments length: expected: 0, given: 1'));
196+
197+
expect(response).toEqual(null);
198+
199+
expect(abstractMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
200+
201+
done();
202+
});
203+
204+
abstractMethod.execute(moduleInstanceMock);
205+
});
206+
174207
it('calls execute and it returns null', async () => {
175208
providerMock.send.mockReturnValueOnce(Promise.resolve(null));
176209

@@ -182,8 +215,6 @@ describe('AbstractMethodTest', () => {
182215

183216
expect(providerMock.send).toHaveBeenCalledWith(abstractMethod.rpcMethod, abstractMethod.parameters);
184217

185-
expect(abstractMethod.callback).toHaveBeenCalledWith(false, null);
186-
187218
expect(abstractMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
188219
});
189220
});

‎packages/web3-eth/src/methods/EthSignMethod.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,16 @@ export default class EthSignMethod extends SignMethod {
6868

6969
if (this.callback) {
7070
this.callback(false, signedMessage);
71+
72+
return;
7173
}
7274

7375
return signedMessage;
7476
} catch (error) {
7577
if (this.callback) {
7678
this.callback(error, null);
79+
80+
return;
7781
}
7882

7983
throw error;

‎packages/web3-eth/tests/src/methods/EthSignMethodTest.js

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,20 @@ describe('EthSignMethodTest', () => {
2929
formatters.inputSignFormatter.mockReturnValue('string');
3030

3131
method = new EthSignMethod(Utils, formatters, moduleInstanceMock);
32-
method.callback = jest.fn();
3332
method.parameters = ['nope', '0x0'];
3433
});
3534

3635
it('constructor check', () => {
3736
expect(method).toBeInstanceOf(SignMethod);
3837
});
3938

40-
it('calls execute with wallets defined', async () => {
39+
it('calls execute with wallets defined and returns a resolved Promise', async () => {
4140
accountsMock.sign.mockReturnValueOnce('0x00');
4241

4342
const response = await method.execute();
4443

4544
expect(response).toEqual('0x00');
4645

47-
expect(method.callback).toHaveBeenCalledWith(false, '0x00');
48-
4946
expect(method.parameters[0]).toEqual('0x0');
5047

5148
expect(method.parameters[1]).toEqual('string');
@@ -57,7 +54,31 @@ describe('EthSignMethodTest', () => {
5754
expect(accountsMock.sign).toHaveBeenCalledWith('string', '0x0');
5855
});
5956

60-
it('calls execute with wallets defined but accounts.sign throws an error', async () => {
57+
it('calls execute with wallets defined and the callback gets called', (done) => {
58+
accountsMock.sign.mockReturnValueOnce('0x00');
59+
60+
method.callback = jest.fn((error, response) => {
61+
expect(error).toEqual(false);
62+
63+
expect(response).toEqual('0x00');
64+
65+
expect(method.parameters[0]).toEqual('0x0');
66+
67+
expect(method.parameters[1]).toEqual('string');
68+
69+
expect(formatters.inputAddressFormatter).toHaveBeenCalledWith('0x0');
70+
71+
expect(formatters.inputSignFormatter).toHaveBeenCalledWith('nope');
72+
73+
expect(accountsMock.sign).toHaveBeenCalledWith('string', '0x0');
74+
75+
done();
76+
});
77+
78+
method.execute();
79+
});
80+
81+
it('calls execute with wallets defined but accounts.sign returns a rejected Promise', async () => {
6182
const error = new Error('SIGN ERROR');
6283
accountsMock.sign = jest.fn(() => {
6384
throw error;
@@ -68,12 +89,27 @@ describe('EthSignMethodTest', () => {
6889
} catch (error2) {
6990
expect(error2).toEqual(error);
7091

71-
expect(method.callback).toHaveBeenCalledWith(error, null);
72-
7392
expect(accountsMock.sign).toHaveBeenCalledWith('string', '0x0');
7493
}
7594
});
7695

96+
it('calls execute with wallets defined but accounts.sign throws an error', (done) => {
97+
const error = new Error('SIGN ERROR');
98+
accountsMock.sign = jest.fn(() => {
99+
throw error;
100+
});
101+
102+
method.callback = jest.fn((error, response) => {
103+
expect(error).toEqual(error);
104+
105+
expect(response).toEqual(null);
106+
107+
done();
108+
});
109+
110+
method.execute();
111+
});
112+
77113
it('calls execute and the account does not exist in the eth-accounts wallet', async () => {
78114
accountsMock.wallet = {nope: {privateKey: '0x0'}};
79115

‎packages/web3-utils/src/Utils.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ export const isAddress = (address, chainId = null) => {
122122
*
123123
* @returns {string} address without prefix
124124
*/
125-
export const stripHexPrefix = (str) => {
126-
return str.slice(0, 2) === '0x' ? str.slice(2) : str
127-
}
125+
export const stripHexPrefix = (string) => {
126+
return string.slice(0, 2) === '0x' ? string.slice(2) : string;
127+
};
128128

129129
/**
130130
* Checks if the given string is a checksummed address
@@ -138,14 +138,14 @@ export const stripHexPrefix = (str) => {
138138
* @returns {Boolean}
139139
*/
140140
export const checkAddressChecksum = (address, chainId = null) => {
141-
const stripAddress = stripHexPrefix(address).toLowerCase()
142-
const prefix = chainId != null ? (chainId.toString() + '0x') : ''
143-
const keccakHash = Hash.keccak256(prefix + stripAddress).toString('hex').replace(/^0x/i, '');
141+
const stripAddress = stripHexPrefix(address).toLowerCase();
142+
const prefix = chainId != null ? chainId.toString() + '0x' : '';
143+
const keccakHash = Hash.keccak256(prefix + stripAddress)
144+
.toString('hex')
145+
.replace(/^0x/i, '');
144146

145147
for (let i = 0; i < stripAddress.length; i++) {
146-
let output = parseInt(keccakHash[i], 16) >= 8 ?
147-
stripAddress[i].toUpperCase() :
148-
stripAddress[i];
148+
let output = parseInt(keccakHash[i], 16) >= 8 ? stripAddress[i].toUpperCase() : stripAddress[i];
149149
if (stripHexPrefix(address)[i] !== output) {
150150
return false;
151151
}

‎packages/web3-utils/src/index.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,17 +256,17 @@ export const toChecksumAddress = (address, chainId = null) => {
256256
throw new Error(`Given address "${address}" is not a valid Ethereum address.`);
257257

258258
const stripAddress = stripHexPrefix(address).toLowerCase();
259-
const prefix = chainId != null ? (chainId.toString() + '0x') : '';
260-
const keccakHash = Hash.keccak256(prefix + stripAddress).toString('hex').replace(/^0x/i, '');
259+
const prefix = chainId != null ? chainId.toString() + '0x' : '';
260+
const keccakHash = Hash.keccak256(prefix + stripAddress)
261+
.toString('hex')
262+
.replace(/^0x/i, '');
261263
let checksumAddress = '0x';
262264

263265
for (let i = 0; i < stripAddress.length; i++)
264-
checksumAddress += parseInt(keccakHash[i], 16) >= 8 ?
265-
stripAddress[i].toUpperCase() :
266-
stripAddress[i];
266+
checksumAddress += parseInt(keccakHash[i], 16) >= 8 ? stripAddress[i].toUpperCase() : stripAddress[i];
267267

268268
return checksumAddress;
269-
}
269+
};
270270

271271
// aliases
272272
export const keccak256 = utils.keccak256;

0 commit comments

Comments
 (0)