Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix: implement backoff retry policy for websocket handler #3600

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions src/chains/ethereum/ethereum/src/forking/handlers/ws-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ export class WsHandler extends BaseHandler implements Handler {
}>
>();

// retry configuration
private retryIntervalBase: number = 3;
private retryCounter: number = 5;
private initialRetryCounter = this.retryCounter;
satyajeetkolhapure marked this conversation as resolved.
Show resolved Hide resolved
private retryTimeoutId: NodeJS.Timeout;

constructor(options: EthereumInternalOptions, abortSignal: AbortSignal) {
super(options, abortSignal);

Expand All @@ -46,10 +52,21 @@ export class WsHandler extends BaseHandler implements Handler {
this.open = this.connect(this.connection, logging);
this.connection.onclose = () => {
satyajeetkolhapure marked this conversation as resolved.
Show resolved Hide resolved
// try to connect again...
// Issue: https://github.com/trufflesuite/ganache/issues/3476
// TODO: backoff and eventually fail
// Issue: https://github.com/trufflesuite/ganache/issues/3477
this.open = this.connect(this.connection, logging);
// backoff and eventually fail
if( this.retryCounter > 0 || this.retryCounter == -1 ) {
satyajeetkolhapure marked this conversation as resolved.
Show resolved Hide resolved
if( this.retryCounter !== -1 ) this.retryCounter--;
clearTimeout( this.retryTimeoutId );
this.retryTimeoutId = setTimeout( () => {
const onCloseEvent = this.connection.onclose;
this.connection = new WebSocket(url.toString(), {
satyajeetkolhapure marked this conversation as resolved.
Show resolved Hide resolved
origin,
headers: this.headers
});
this.connection.binaryType = "nodebuffer";
this.connection.onclose = onCloseEvent;
this.open = this.connect(this.connection, logging);
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also a inFlightRequests that may need care/attention here. I am not sure if the desired behavior would be to kill/cancel them or attempt to rebroadcast if/once the socket reconnects and if that's possible. I don't know this part of code. I would assume the latter but @davidmurdoch will need to clarify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @davidmurdoch : Will this change have any impact on inFlightRequests?

Copy link
Contributor Author

@satyajeetkolhapure satyajeetkolhapure Dec 13, 2022

Choose a reason for hiding this comment

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

@tenthirtyone , @davidmurdoch : In flight requests are now stored when connection is closed and processed once connection retry attempt succeeds

}, Math.pow( this.retryIntervalBase, this.initialRetryCounter - this.retryCounter ) * 1000 );
}
};
this.abortSignal.addEventListener("abort", () => {
this.connection.onclose = null;
Expand Down Expand Up @@ -117,6 +134,8 @@ export class WsHandler extends BaseHandler implements Handler {
() => {
connection.onopen = null;
connection.onerror = null;
// reset the retry counter
this.retryCounter = this.initialRetryCounter;
},
err => {
logging.logger.log(err);
Expand Down