Skip to content

Commit

Permalink
catch unhandled error when reconnecting with request queue (#7141)
Browse files Browse the repository at this point in the history
* remove todo

* add erorr handling

* add tests

* fix lint

* format

* update changelog

---------

Co-authored-by: Alex Luu <[email protected]>
  • Loading branch information
luu-alex and Alex Luu authored Jul 8, 2024
1 parent 59e67bf commit 38fac06
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 41 deletions.
16 changes: 9 additions & 7 deletions packages/web3-utils/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
## [Unreleased]

- `_sendPendingRequests` will catch unhandled errors from `_sendToSocket` (#6968)
6 changes: 2 additions & 4 deletions packages/web3-utils/src/event_emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/
/* eslint-disable max-classes-per-file */

import EventEmitter3 from 'eventemitter3';

import EventEmitter3 from 'eventemitter3';

/**
* This class copy the behavior of Node.js EventEmitter class.
Expand All @@ -35,5 +34,4 @@ export class EventEmitter extends EventEmitter3 {
public getMaxListeners(): number {
return this.maxListeners;
}

}
}
14 changes: 10 additions & 4 deletions packages/web3-utils/src/socket_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -503,9 +503,15 @@ export abstract class SocketProvider<

private _sendPendingRequests() {
for (const [id, value] of this._pendingRequestsQueue.entries()) {
this._sendToSocket(value.payload as Web3APIPayload<API, any>);
this._pendingRequestsQueue.delete(id);
this._sentRequestsQueue.set(id, value);
try {
this._sendToSocket(value.payload as Web3APIPayload<API, any>);
this._pendingRequestsQueue.delete(id);
this._sentRequestsQueue.set(id, value);
} catch (error) {
// catches if sendTosocket fails
this._pendingRequestsQueue.delete(id);
this._eventEmitter.emit('error', error);
}
}
}

Expand Down
1 change: 0 additions & 1 deletion packages/web3-utils/src/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) &&
Expand Down
15 changes: 4 additions & 11 deletions packages/web3-utils/test/fixtures/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
{}
];
export const isContractInitInvalidData: unknown[] = ['', 12, {}];
29 changes: 28 additions & 1 deletion packages/web3-utils/test/unit/socket_provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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', () => {
Expand Down
22 changes: 9 additions & 13 deletions packages/web3-utils/test/unit/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
compareBlockNumbersInvalidData,
compareBlockNumbersValidData,
isContractInitValidData,
isContractInitInvalidData
isContractInitInvalidData,
} from '../fixtures/validation';

describe('validation', () => {
Expand All @@ -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);
});
});
});
});

1 comment on commit 38fac06

@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: 38fac06 Previous: 59e67bf Ratio
processingTx 8674 ops/sec (±4.68%) 8895 ops/sec (±4.11%) 1.03
processingContractDeploy 40592 ops/sec (±6.80%) 39951 ops/sec (±7.33%) 0.98
processingContractMethodSend 16400 ops/sec (±6.88%) 16346 ops/sec (±9.26%) 1.00
processingContractMethodCall 27278 ops/sec (±7.84%) 28343 ops/sec (±6.15%) 1.04
abiEncode 45234 ops/sec (±7.24%) 44357 ops/sec (±8.20%) 0.98
abiDecode 31124 ops/sec (±7.73%) 32292 ops/sec (±6.23%) 1.04
sign 1589 ops/sec (±0.76%) 1544 ops/sec (±4.56%) 0.97
verify 369 ops/sec (±2.66%) 377 ops/sec (±0.41%) 1.02

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

Please sign in to comment.