web3.js icon indicating copy to clipboard operation
web3.js copied to clipboard

Issues in Contract subscriptions

Open Muhammad-Altabba opened this issue 2 years ago • 0 comments

The user can write: contractInstance.subscriptionManager.subscribe('logs') but it throws an error. The question is: If this is a non-supported way of subscribing to contract events, then should it be prevented at typescript compile time? Or is a supported but throws an error?

We need to answer the question above and write tests for contract subscriptions.

Code snippet that produce the error

Consider the following test:

describe('contract subscriptions', () => {
it('do not throw', async () => {
	const contractInstance = new Contract(
		abi,
		'0x407d73d8a49eeb85d32cf465507dd71d507100c1',
	);

	// the point is: this is supported code but it throws the following exception:
	//	TypeError: Cannot read properties of undefined (reading 'address')
	// at new ContractLogsSubscription (/home/maltabba/repos/web3v4.x/packages/web3-eth-contract/src/contract_log_subscription.ts:354:25)
	// at Web3SubscriptionManager.<anonymous> (/home/maltabba/repos/web3v4.x/packages/web3-core/src/web3_subscription_manager.ts:163:24)
	
	// the error indicate that the abi and address provided to the contract constructor
	//	is not passed to the subscription instance
	expect(contractInstance.subscriptionManager.subscribe('logs')).toBeInstanceOf(Promise);

	// the one that is supposed to work is: contractInstance.events.allEvents() but it needs to be
        //   tested and documented as well.
});
});

image

Additionally

The class LogsSubscription inside web3-eth-contract has a confusing name as there is already a class named the same at web3-eth. So the class inside web3-eth-contract should be renamed to something like: ContractLogsSubscription and the exported name should be marked as absolute. This change is drafted here: https://github.com/web3/web3.js/pull/6285/commits/366dbed8b80c4217010c59d5adbfbb396aaa5a7e

Environment

web3.js version 4.x

Muhammad-Altabba avatar Jul 17 '23 08:07 Muhammad-Altabba