Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manage provider timeouts #678

Closed
mrwillis opened this issue Dec 8, 2019 · 15 comments
Closed

Manage provider timeouts #678

mrwillis opened this issue Dec 8, 2019 · 15 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@mrwillis
Copy link

mrwillis commented Dec 8, 2019

Hi Richard,

Is it possible to manage timeouts from the provider? I currently use this code to create a "retry provider" for certain status codes. Is it possible to do the same for timeouts?

import { JsonRpcProvider } from "ethers/providers";
import { Networkish } from "ethers/utils";
import { ConnectionInfo, poll } from "ethers/utils/web";

export class RetryProvider extends JsonRpcProvider {
  public attempts: number;

  constructor(
    attempts: number,
    url?: ConnectionInfo | string,
    network?: Networkish
  ) {
    super(url, network);
    this.attempts = attempts;
  }

  public perform(method: string, params: any) {
    let attempts = 0;
    return poll(() => {
      attempts++;
      return super.perform(method, params).then(
        result => {
          return result;
        },
        (error: any) => {
          if (
            (error.statusCode === 429 || error.statusCode > 499) &&
            attempts < this.attempts
          ) {
            return Promise.resolve(undefined);
          } else {
            return Promise.reject(error);
          }
        }
      );
    });
  }
}

We deal with this in particular a fair bit:

2019-12-08 13:03:36 error: Error: timeout at Timeout._onTimeout (/var/app/current/node_modules/ethers/utils/web.js:54:20) at ontimeout (timers.js:436:11) at tryOnTimeout (timers.js:300:5) at listOnTimeout (timers.js:263:5) at Timer.processTimers (timers.js:223:10)

@ricmoo
Copy link
Member

ricmoo commented Dec 9, 2019

Is this in v4 or v5? For the FallbackProvider, I plan to add a shorter timeout for trying the next in the list of providers.

And for throttling status codes, retry with exponential back off is being added. Are these the status codes you are targeting?

@mrwillis
Copy link
Author

mrwillis commented Dec 9, 2019

@ricmoo This is v4. I already implemented exponential backoff above for 4XX but I'm wondering if exponential backoff can be done with HTTP timeouts as well.

@krzkaczor
Copy link

I use JsonRpcProvider with forked ganache which makes it SUPER slow. Unfortunately some requests timeout :( It would be cool to bump timeout for such case. (option for a provider perhaps?)

@ricmoo
Copy link
Member

ricmoo commented Feb 24, 2020

How slow is slow? Why is it slow? If it is a PoA network, you should be able to reduce the pollingInterval to speed it up. It should never timeout... :s

@krzkaczor
Copy link

It's slow because forked ganache proxies calls to external node. I do gas estimation.

I found hardcoded timeout in web.ts and I am adjusting it now. I will keep you posted.

@adrianmcli
Copy link

@krzkaczor Did you figure out how to get around these timeouts?

@krzkaczor
Copy link

@adrianmcli not really - I suspect that ganache drops the connection, so it's not ethers.js fault.

Depending on your case changing this hardcoded timeout in web.ts can work for you...

@adrianmcli
Copy link

@krzkaczor interesting, it seems that doing this gets around it:

try {
  await myContract.execute(address, callbackData, { gasLimit: 4000000 });
} catch (e) {
  if (
    e
      .toString()
      .toLowerCase()
      .includes("timeout")
  ) {
    await sleep(60 * 1000); // sleep for extra minute after timeout
  } else {
    throw e;
  }
}

I believe this is the hard-coded timeout you are referring to: https://github.com/ethers-io/ethers.js/blob/master/src.ts/utils/web.ts#L44

I also agree that we should be able to easily configure this when we create the provider.

@timoxley
Copy link

timoxley commented May 18, 2020

Seeing this a lot too, most frustrating part is that it doesn't tell you what actually timed out, just that something did, due to minimally helpful Error object here:

reject(new Error('timeout'));


doing this gets around it

@adrianmcli as far as I can tell this doesn't actually do anything about the problem though, it gets around it by suppressing the error and then waiting for a magic minute. Doesn't seem very robust.

@ricmoo
Copy link
Member

ricmoo commented May 18, 2020

I can add the URL to the error. I’ll see what else can be added. I would likely only
Change this in v5 though.

The error should be on some promise being waited for, so I would think the web fetch would be further down in the stack, so not very useful in general? Not as much as the .catch on the timed out promise.

I’d have to look back at v4 more deeply, but I think you can specify this in the connection object. You can in v5, when connecting, just specify the timeout property on the connection object.

@ricmoo
Copy link
Member

ricmoo commented May 18, 2020

Just looking at the source code for v4, looks like passing in a timeout to the ConnectionInfo object should work fine for setting the timeout.

@timoxley
Copy link

timoxley commented May 19, 2020

The error should be on some promise being waited for, so I would think the web fetch would be further down in the stack, so not very useful in general? Not as much as the .catch on the timed out promise.

@ricmoo The problem I'm seeing is that there's no stack for figuring out which command actually failed after the fact, this is all that appears:

Error: timeout
    at Timeout._onTimeout (/home/node/node_modules/ethers/utils/web.js:54:20)
    at listOnTimeout (internal/timers.js:549:17)
    at processTimers (internal/timers.js:492:7)

I'm not sure this is improved unless I start putting catch handlers that augment the error with my own message around every async request?

@ricmoo ricmoo added discussion Questions, feedback and general information. enhancement New feature or improvement. on-deck This Enhancement or Bug is currently being worked on. and removed discussion Questions, feedback and general information. labels Jun 6, 2020
@ricmoo
Copy link
Member

ricmoo commented Jun 12, 2020

The latest beta version should include a lot more details on timeouts now. Let me know if it helps you. It includes the URL and request body now...

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Jun 12, 2020
@ricmoo
Copy link
Member

ricmoo commented Jul 5, 2020

I think the error now contains enough extra detail about what timed out that debugging should be much easier, so I'm going to close this. If not though, please feel free to re-open.

Thanks! :)

@ricmoo ricmoo closed this as completed Jul 5, 2020
@oxdog
Copy link

oxdog commented May 14, 2022

Might help someone: I used ganache fork (8.0) w/ JsonProviderRPC and for me the issue was that I needed a lot of data from a contract view function. The way ganache fork works is taking a snapshot of the chain and only loading relevant data when you make a transaction. When you need a lot of relevant data it takes time. Reducing the data needed resolved the issue in my case (I needed about 5k values from different contracts, for testing purpose reduced it to 100)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

6 participants