Web3.py: Add retry middleware

[kevinday]

What was wrong?

It's not uncommon for requests sent over the network to fail due to network interruptions, and in most cases retrying the request will resolve the issue.

How can it be fixed?

Implement simple retry middleware to handle retry on HTTP errors automatically.

[carver]

It might make sense for this to be a default middleware attached to the HTTPProvider.

[bweick]

Hey guys...if no one has claimed this one I wouldn't mind trying it out. Are there any checks needed for the quality of data returned (i.e. get a 200 code but the data field has bad data in it)? Or should it be straight forward, as long as the make_request function doesn't error out, we can reliably return the response?

[pipermerriam]

should it be straight forward, as long as the make_request function doesn't error out, we can reliably return the response?

I think it should be something as simple as a try/except block within a loop that retries up to N times, catching and silencing the appropriate timeout/connection-error/whatever exceptions.

for i in range(N):
    try:
        return make_request(...)
    except (WhateverTimeoutError, WhateverConnectionError, WhateverOtherError):
        if i < N - 1:  # not on the last iteration of the loop.
            continue
        else:
            raise

Also, it should have either a whitelist (preferred), or a blacklist of what RPC methods it will retry. For example, it should not ever retry eth_sendTransaction (unless maybe the nonce field was specified) otherwise we risk sending the same transaction multiple times.

[bweick]

Cool my implementation to check the response is pretty much identical to yours. As for implementing the whitelist I decided instead of listing out all the individual methods that some whole "classes" of methods (admin, miner, shh, net, txpool, testing, evm) could be approved without worry, let me know if you a) think that's a bad assumption, or b) don't like the idea because it doesn't require all allowable functions to be explicitly declared.

As for the eth and personal method "classes" I left off those pertaining to sending transactions (eth_sendTransaction, eth_sendRawTransaction, personal_signAndSendTransaction, personal_sendTransaction) and individually listed all other methods.

In order to see if the method is contained in the whitelist I created a function that returns True or False and checks to see if the function is part of one of the classes that have full approval and then if not, check if it is one of the approved fxns within either personal or eth (see code below for clarity)

 def check_method(method):
        root = method.split('_')[0]
        if root in whitelist:
            return True
        elif method in whitelist:
            return True
        else:
            return False

I'll get a PR written up tomorrow and have that submitted as well.

As a side note...I'm having a hell of a time being able to run all the tests since I can't seem to get pkg-config to work on my computer. I know you guys are looking at getting rid of the dependency that needs it. But anything I can do to help with that as well I'm more than willing to lend a hand cause it kinda blocks up my development in general.

[carver]

Seems like a good approach. eth_sendRawTransaction should be idempotent at the node, so retries on that should be okay, too.

[carver]

I can't seem to get pkg-config to work on my computer

This on Windows, I presume? Hopefully #505 will help here. I will be pushing that forward a bit more in the next few days.

[bweick]

Yeah on Windows, saw y'all were doing some work on that so hopefully won't be an issue for much longer...finally got around it by running a Linux subsystem. Should have the tests done by today and I'll put up the PR.

[bweick]

With #533 and #534 merged this can be closed I think.