WebSocket reconnect doesn't throw error on maxAttempts

[angel200sdnot]

Expected behavior

An error should be thrown when the max attempts is reached while reconnecting. Next time a method gets called that requires this connection then it should try to reconnect again.

Actual behavior

WebSocket continues to try to reconnect until it succeeds. This only happens after the initial connection is successful. When the initial connection fails then the reconnection works as expected

Steps to reproduce the behavior

1. Create Web3 instance using WebSocket provider with the following configuration

{
      reconnect: {
        auto: true,
        delay: 1000, // ms
        onTimeout: false,
        maxAttempts: 1
      }
}

2. Attempt to get a transaction receipt
3. Observe that the transaction receipt is received
4. Disconnect your internet
5. Attempt to get same transaction receipt
6. Wait for a minute
7. Reconnect internet
8. Observe that the transaction receipt is received

I noticed that this also happens even when auto is set to false. I was expecting to get the following error instead: Error: CONNECTION TIMEOUT: timeout of 5000 ms achived

Versions

Node: v12.16.2
Web3.js: 1.2.7
OS: Ubuntu 18.04.4 LTS

[cgewecke]

@angel200sdnot Thanks for reporting. Will investigate

We have a some tests that seem to cover the case you describe but we need to look at the interaction with delay and timeout more closely. And make sure the error is propagating to methods, etc..

• basic check of max reconnections hit error

[cgewecke]

@angel200sdnot Have opened #3494 to address this. If you have a chance, could you look at the tests added there and see if they model your expectations?

In the issue description you say:

An error should be thrown when the max attempts is reached while reconnecting. Next time a method gets called that requires this connection then it should try to reconnect again.

At present, an error is emitted for the provider when max attempts is reached. You can listen for it by attaching a handler on the provider as shown here:

https://github.com/ethereum/web3.js/blob/bcf248f895fe14d466e6fe60e9096f0fc5e7a766/test/websocket.ganache.js#L318-L321

#3494 makes sure subsequent requests error with connection not open on send(). But to establish the connection again after max attempts have been exhausted you have to call the connect method manually.

Does that seem correct?

[angel200sdnot]

@cgewecke This is what i currently have in place for reconnecting using web3@1.2.6

async function getTransactionReceipt(transactionHash: string): Promise<any> {
    try {
      const result = await this.web3.eth.getTransactionReceipt(transactionHash);
      return result;
    } catch (err) {
      if (err && err.message === 'connection not open') {
        this.connectWeb3();
        return this.web3.eth.getTransactionReceipt(transactionHash);
      }
      throw err;
    }
}

With web3@1.2.7 i was expecting to be able to do the same by using

{
  reconnect: {
    auto: true,
    maxAttempts: 1,
    delay: 0
  }
}

#3494 makes sure subsequent requests error with connection not open on send(). But to establish the connection again after max attempts have been exhausted you have to call the connect method manually.

In my opinion this doesn't seem correct because in that case i would need to keep handling the error and reconnecting manually.

Perhaps i am looking at this the wrong way and i should be handling this differently, do you have any suggestions regarding my approach?

[angel200sdnot]

I tested my code by disconnecting my internet for a couple of minutes and then reconnecting and getTransactionReceipt seems to be working fine without the error handling. I did that reconnection logic because i was working under the assumption that the WebsocketProvider connection closes after being idle but now i'm not sure if thats true (i had connection not open error when fetching transaction receipt a day after the initial connection was made)

If that was unrelated to reconnecting then this issue can be closed with #3494

[cgewecke]

@angel200sdnot Ah, the idea of max attempts is that it's like running your initial code in a loop, where max attempts is the upper bound. There is a bug...I think #3494 will resolve this though.

Thanks so much.