Skip to content

Commit

Permalink
catching promise - legacy provider error (#7022)
Browse files Browse the repository at this point in the history
* adding data check for revert util method

* fix up tests

* fix lint

* change legacy request

* revert contract changes, add try catch

* fix lint

* fix changes

* update tests

* lint and feedback

* update test

* update test

* format
  • Loading branch information
Alex committed May 14, 2024
1 parent 0408125 commit 866469d
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 14 deletions.
21 changes: 18 additions & 3 deletions packages/web3-core/src/web3_request_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ export class Web3RequestManager<
) as JsonRpcPayload;

if (!isNullish(this.middleware)) {
payload = (await this.middleware.processRequest(payload));
payload = await this.middleware.processRequest(payload);
}
if (isWeb3Provider(provider)) {
let response;
Expand Down Expand Up @@ -248,7 +248,7 @@ export class Web3RequestManager<
// TODO: This could be deprecated and removed.
if (isLegacyRequestProvider(provider)) {
return new Promise<JsonRpcResponse<ResponseType>>((resolve, reject) => {
const rejectWithError = (err: unknown) =>
const rejectWithError = (err: unknown) => {
reject(
this._processJsonRpcResponse(
payload,
Expand All @@ -259,6 +259,8 @@ export class Web3RequestManager<
},
),
);
};

const resolveWithResponse = (response: JsonRpcResponse<ResponseType>) =>
resolve(
this._processJsonRpcResponse(payload, response, {
Expand Down Expand Up @@ -288,7 +290,20 @@ export class Web3RequestManager<
const responsePromise = result as unknown as Promise<
JsonRpcResponse<ResponseType>
>;
responsePromise.then(resolveWithResponse).catch(rejectWithError);
responsePromise.then(resolveWithResponse).catch(error => {
try {
// Attempt to process the error response
const processedError = this._processJsonRpcResponse(
payload,
error as JsonRpcResponse<ResponseType, unknown>,
{ legacy: true, error: true },
);
reject(processedError);
} catch (processingError) {
// Catch any errors that occur during the error processing
reject(processingError);
}
});
}
});
}
Expand Down
33 changes: 33 additions & 0 deletions packages/web3-core/test/unit/web3_request_manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ describe('Web3RequestManager', () => {
expect(myProvider.request).toHaveBeenCalledTimes(1);
expect(await pr).toBe('test');
});

it('Got a "nullish" response from provider', async () => {
const manager = new Web3RequestManager(undefined, undefined);
const myProvider = {
Expand Down Expand Up @@ -1134,6 +1135,38 @@ describe('Web3RequestManager', () => {
expect(myProvider.request).toHaveBeenCalledWith(payload, expect.any(Function));
});

it('should error in isPromise', async () => {
const manager = new Web3RequestManager();
const myProvider = {
request: jest
.fn()
.mockImplementation(async () => Promise.reject(errorResponse)),
} as any;

jest.spyOn(manager, 'provider', 'get').mockReturnValue(myProvider);

await expect(manager.sendBatch(request)).rejects.toEqual(errorResponse);
expect(myProvider.request).toHaveBeenCalledTimes(1);
expect(myProvider.request).toHaveBeenCalledWith(payload, expect.any(Function));
});

it('should catch error and process json response in isPromise', async () => {
const manager = new Web3RequestManager();
const myProvider = {
request: jest
.fn()
.mockImplementation(async () => Promise.reject(errorResponse)),
} as any;
jest.spyOn(manager as any, '_processJsonRpcResponse').mockImplementation(() => {
return errorResponse;
});
jest.spyOn(manager, 'provider', 'get').mockReturnValue(myProvider);

await expect(manager.sendBatch(request)).rejects.toEqual(errorResponse);
expect(myProvider.request).toHaveBeenCalledTimes(1);
expect(myProvider.request).toHaveBeenCalledWith(payload, expect.any(Function));
});

it('should pass request to provider and reject if provider returns error', async () => {
const manager = new Web3RequestManager();
const myProvider = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ describe('Web3Eth.sendTransaction', () => {
expect(response.status).toBe(BigInt(1));
expect(response.events).toBeUndefined();

const minedTransactionData = await web3EthWithWallet.getTransaction(response.transactionHash);
const minedTransactionData = await web3EthWithWallet.getTransaction(
response.transactionHash,
);

expect(minedTransactionData).toMatchObject({
from: tempAcc.address,
Expand Down Expand Up @@ -119,7 +121,9 @@ describe('Web3Eth.sendTransaction', () => {
expect(response.status).toBe(BigInt(1));
expect(response.events).toBeUndefined();

const minedTransactionData = await web3EthWithWallet.getTransaction(response.transactionHash);
const minedTransactionData = await web3EthWithWallet.getTransaction(
response.transactionHash,
);

expect(minedTransactionData).toMatchObject({
from: tempAcc.address,
Expand Down Expand Up @@ -152,7 +156,9 @@ describe('Web3Eth.sendTransaction', () => {
expect(response.status).toBe(BigInt(1));
expect(response.events).toBeUndefined();

const minedTransactionData = await web3EthWithWallet.getTransaction(response.transactionHash);
const minedTransactionData = await web3EthWithWallet.getTransaction(
response.transactionHash,
);

expect(minedTransactionData).toMatchObject({
from: tempAcc.address,
Expand Down Expand Up @@ -534,9 +540,12 @@ describe('Web3Eth.sendTransaction', () => {
from: tempAcc.address,
data: SimpleRevertDeploymentData,
};
simpleRevertDeployTransaction.gas = await web3Eth.estimateGas(simpleRevertDeployTransaction);
simpleRevertContractAddress = (await web3Eth.sendTransaction(simpleRevertDeployTransaction))
.contractAddress as Address;
simpleRevertDeployTransaction.gas = await web3Eth.estimateGas(
simpleRevertDeployTransaction,
);
simpleRevertContractAddress = (
await web3Eth.sendTransaction(simpleRevertDeployTransaction)
).contractAddress as Address;
});

it('Should throw TransactionRevertInstructionError because gas too low', async () => {
Expand Down Expand Up @@ -578,7 +587,9 @@ describe('Web3Eth.sendTransaction', () => {
const transaction: Transaction = {
from: tempAcc.address,
to: '0x0000000000000000000000000000000000000000',
value: BigInt('99999999999999999999999999999999999999999999999999999999999999999'),
value: BigInt(
'99999999999999999999999999999999999999999999999999999999999999999',
),
};

const expectedThrownError = {
Expand All @@ -587,7 +598,9 @@ describe('Web3Eth.sendTransaction', () => {
code: 402,
reason:
getSystemTestBackend() === BACKEND.GETH
? expect.stringContaining('err: insufficient funds for gas * price + value: address')
? expect.stringContaining(
'err: insufficient funds for gas * price + value: address',
)
: 'VM Exception while processing transaction: insufficient balance',
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,16 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.
// eslint-disable-next-line import/no-extraneous-dependencies
import hardhat from 'hardhat';

import { performBasicRpcCalls } from './helper';
import { performBasicRpcCalls, failErrorCalls} from './helper';

describe('compatibility with `hardhat` provider', () => {
it('should initialize Web3, get accounts & block number and send a transaction', async () => {
// use the hardhat provider for web3.js
await performBasicRpcCalls(hardhat.network.provider);
await expect(performBasicRpcCalls(hardhat.network.provider)).resolves.not.toThrow();
});
it('should throw on error calls', async () => {
const result = failErrorCalls(hardhat.network.provider);
await expect(result).rejects.toThrow();

})
});
28 changes: 27 additions & 1 deletion packages/web3/test/integration/external-providers/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

import { SupportedProviders } from 'web3-types';
import { Contract } from 'web3-eth-contract';
import Web3 from '../../../src/index';
import { BasicAbi, BasicBytecode } from '../../shared_fixtures/build/Basic';

/**
* Performs basic RPC calls (like `eth_accounts`, `eth_blockNumber` and `eth_sendTransaction`)
Expand All @@ -36,7 +38,7 @@ export async function performBasicRpcCalls(provider: SupportedProviders) {
to: accounts[1],
from: accounts[0],
value: '1',
gas: 21000
gas: 21000,
});
expect(tx.status).toBe(BigInt(1));

Expand All @@ -46,3 +48,27 @@ export async function performBasicRpcCalls(provider: SupportedProviders) {
// After sending a transaction, the blocknumber is supposed to be greater than or equal the block number before sending the transaction
expect(blockNumber1).toBeGreaterThanOrEqual(blockNumber0);
}

export async function failErrorCalls(provider: SupportedProviders) {
let contract: Contract<typeof BasicAbi>;
const web3 = new Web3(provider);

contract = new web3.eth.Contract(BasicAbi, undefined, {
provider,
});

let deployOptions: Record<string, unknown>;

// eslint-disable-next-line prefer-const
deployOptions = {
data: BasicBytecode,
arguments: [10, 'string init value'],
};
const accounts = await web3.eth.getAccounts();

const sendOptions = { from: accounts[0], gas: '1000000' };

contract = await contract.deploy(deployOptions).send(sendOptions);

await contract.methods.reverts().send({ from: accounts[0] });
}

1 comment on commit 866469d

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 866469d Previous: 0408125 Ratio
processingTx 8892 ops/sec (±3.98%) 9033 ops/sec (±3.91%) 1.02
processingContractDeploy 38535 ops/sec (±6.62%) 39144 ops/sec (±7.12%) 1.02
processingContractMethodSend 18875 ops/sec (±7.64%) 18833 ops/sec (±7.39%) 1.00
processingContractMethodCall 40251 ops/sec (±5.97%) 37969 ops/sec (±6.28%) 0.94
abiEncode 42727 ops/sec (±6.75%) 42697 ops/sec (±7.11%) 1.00
abiDecode 29052 ops/sec (±7.20%) 29364 ops/sec (±7.78%) 1.01
sign 1560 ops/sec (±0.80%) 1555 ops/sec (±0.74%) 1.00
verify 371 ops/sec (±0.49%) 357 ops/sec (±2.74%) 0.96

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.