From 38fac0661968e609bf06249286897dcdf9e421a4 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 8 Jul 2024 12:55:30 -0400 Subject: [PATCH] catch unhandled error when reconnecting with request queue (#7141) * remove todo * add erorr handling * add tests * fix lint * format * update changelog --------- Co-authored-by: Alex Luu --- packages/web3-utils/CHANGELOG.md | 16 +++++----- packages/web3-utils/src/event_emitter.ts | 6 ++-- packages/web3-utils/src/socket_provider.ts | 14 ++++++--- packages/web3-utils/src/validation.ts | 1 - .../web3-utils/test/fixtures/validation.ts | 15 +++------- .../test/unit/socket_provider.test.ts | 29 ++++++++++++++++++- .../web3-utils/test/unit/validation.test.ts | 22 ++++++-------- 7 files changed, 62 insertions(+), 41 deletions(-) diff --git a/packages/web3-utils/CHANGELOG.md b/packages/web3-utils/CHANGELOG.md index 5fcf44ccec5..20474ce0ab3 100644 --- a/packages/web3-utils/CHANGELOG.md +++ b/packages/web3-utils/CHANGELOG.md @@ -213,19 +213,21 @@ Documentation: ### Fixed -- fixed toHex incorrectly hexing Uint8Arrays and Buffer (#6957) -- fixed isUint8Array not returning true for Buffer (#6957) +- fixed toHex incorrectly hexing Uint8Arrays and Buffer (#6957) +- fixed isUint8Array not returning true for Buffer (#6957) ## [4.3.0] ### Added -- `toWei` add warning when using large numbers or large decimals that may cause precision loss (#6908) -- `toWei` and `fromWei` now supports integers as a unit. (#7053) +- `toWei` add warning when using large numbers or large decimals that may cause precision loss (#6908) +- `toWei` and `fromWei` now supports integers as a unit. (#7053) ### Fixed -- `toWei` support numbers in scientific notation (#6908) -- `toWei` and `fromWei` trims according to ether unit successfuly (#7044) +- `toWei` support numbers in scientific notation (#6908) +- `toWei` and `fromWei` trims according to ether unit successfuly (#7044) -## [Unreleased] \ No newline at end of file +## [Unreleased] + +- `_sendPendingRequests` will catch unhandled errors from `_sendToSocket` (#6968) diff --git a/packages/web3-utils/src/event_emitter.ts b/packages/web3-utils/src/event_emitter.ts index bc7fee36f04..54a2610da28 100644 --- a/packages/web3-utils/src/event_emitter.ts +++ b/packages/web3-utils/src/event_emitter.ts @@ -16,8 +16,7 @@ along with web3.js. If not, see . */ /* eslint-disable max-classes-per-file */ -import EventEmitter3 from 'eventemitter3'; - +import EventEmitter3 from 'eventemitter3'; /** * This class copy the behavior of Node.js EventEmitter class. @@ -35,5 +34,4 @@ export class EventEmitter extends EventEmitter3 { public getMaxListeners(): number { return this.maxListeners; } - -} \ No newline at end of file +} diff --git a/packages/web3-utils/src/socket_provider.ts b/packages/web3-utils/src/socket_provider.ts index 0a73fb8378f..52154402ccd 100644 --- a/packages/web3-utils/src/socket_provider.ts +++ b/packages/web3-utils/src/socket_provider.ts @@ -426,7 +426,7 @@ export abstract class SocketProvider< this._reconnectAttempts += 1; setTimeout(() => { this._removeSocketListeners(); - this.connect(); + this.connect(); // this can error out this.isReconnecting = false; }, this._reconnectOptions.delay); } else { @@ -503,9 +503,15 @@ export abstract class SocketProvider< private _sendPendingRequests() { for (const [id, value] of this._pendingRequestsQueue.entries()) { - this._sendToSocket(value.payload as Web3APIPayload); - this._pendingRequestsQueue.delete(id); - this._sentRequestsQueue.set(id, value); + try { + this._sendToSocket(value.payload as Web3APIPayload); + this._pendingRequestsQueue.delete(id); + this._sentRequestsQueue.set(id, value); + } catch (error) { + // catches if sendTosocket fails + this._pendingRequestsQueue.delete(id); + this._eventEmitter.emit('error', error); + } } } diff --git a/packages/web3-utils/src/validation.ts b/packages/web3-utils/src/validation.ts index 76f1de97742..57e91aae76d 100644 --- a/packages/web3-utils/src/validation.ts +++ b/packages/web3-utils/src/validation.ts @@ -173,7 +173,6 @@ export const compareBlockNumbers = (blockA: BlockNumberOrTag, blockB: BlockNumbe return 1; }; - export const isContractInitOptions = (options: unknown): options is ContractInitOptions => typeof options === 'object' && !isNullishValidator(options) && diff --git a/packages/web3-utils/test/fixtures/validation.ts b/packages/web3-utils/test/fixtures/validation.ts index 3c216d3ff62..c170f5ed813 100644 --- a/packages/web3-utils/test/fixtures/validation.ts +++ b/packages/web3-utils/test/fixtures/validation.ts @@ -79,16 +79,9 @@ export const isBloomValidData: [any, true][] = [ ]; export const isContractInitValidData: ContractInitOptions[] = [ - {dataInputFill: "data"}, - {syncWithContext: true}, - {gas: "100000", - syncWithContext: true, - dataInputFill: "data", - }, + { dataInputFill: 'data' }, + { syncWithContext: true }, + { gas: '100000', syncWithContext: true, dataInputFill: 'data' }, ]; -export const isContractInitInvalidData: unknown[] = [ - "", - 12, - {} -]; \ No newline at end of file +export const isContractInitInvalidData: unknown[] = ['', 12, {}]; diff --git a/packages/web3-utils/test/unit/socket_provider.test.ts b/packages/web3-utils/test/unit/socket_provider.test.ts index c2c3756c755..19529f4181a 100644 --- a/packages/web3-utils/test/unit/socket_provider.test.ts +++ b/packages/web3-utils/test/unit/socket_provider.test.ts @@ -252,7 +252,6 @@ describe('SocketProvider', () => { expect(deleteSpy).toHaveBeenCalledTimes(sentRequestsQueueSize); }); }); - describe('testing connect() method', () => { it('should call method reconnect in case of error at _openSocketConnection', async () => { const provider = new TestProvider(socketPath, socketOption); @@ -496,6 +495,34 @@ describe('SocketProvider', () => { expect(deleteSpy).toHaveBeenCalled(); }); }); + describe('testing _onConnect() method', () => { + it('should catch error when succesfully connecting with _sendPendingRequests in queue and _sendToSocket throws', async () => { + const provider = new TestProvider(socketPath, socketOption, { delay: 0 }); + provider.setStatus('connecting'); + const payload1 = { id: 1, method: 'some_rpc_method' }; + const errorEventSpy = jest.fn(); + provider.on('error', errorEventSpy); + + // @ts-expect-error access protected method + provider._sendToSocket = () => { + throw new Error('any error'); + }; + provider + .request(payload1) + .then(() => { + // nothing + }) + .catch(() => { + // nothing + }); + // @ts-expect-error run protected method + provider._onConnect(); + + expect(errorEventSpy).toHaveBeenCalledWith(expect.any(Error)); + expect(provider.getPendingRequestQueueSize()).toBe(0); + expect(provider.getSentRequestsQueueSize()).toBe(0); + }); + }); describe('testing _clearQueues() method', () => { it('should clear queues when called', () => { diff --git a/packages/web3-utils/test/unit/validation.test.ts b/packages/web3-utils/test/unit/validation.test.ts index 9d087e3a2eb..1cbfb389466 100644 --- a/packages/web3-utils/test/unit/validation.test.ts +++ b/packages/web3-utils/test/unit/validation.test.ts @@ -23,7 +23,7 @@ import { compareBlockNumbersInvalidData, compareBlockNumbersValidData, isContractInitValidData, - isContractInitInvalidData + isContractInitInvalidData, } from '../fixtures/validation'; describe('validation', () => { @@ -40,19 +40,15 @@ describe('validation', () => { ); }); describe('isContractInit', () => { - describe('should return true', () => { - it.each([...isContractInitValidData])( - '%s', (input) => { - expect(isContractInitOptions(input)).toBe(true); - } - ) + describe('should return true', () => { + it.each([...isContractInitValidData])('%s', input => { + expect(isContractInitOptions(input)).toBe(true); + }); }); - describe('should return false', () => { - it.each([...isContractInitInvalidData])( - '%s', (input) => { - expect(isContractInitOptions(input)).toBe(false); - } - ) + describe('should return false', () => { + it.each([...isContractInitInvalidData])('%s', input => { + expect(isContractInitOptions(input)).toBe(false); + }); }); }); });