Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Retry on failure #28

Closed
tomwys opened this Issue · 8 comments

2 participants

@tomwys

Recently Facebook often returns following error on my requests:

{'error_code': 1, 'error_msg': 'An unknown error occurred'}

If I re-execute query, it starts working. So my proposition is to implement GraphAPI with retry option.

I think that there should be implemented two classes that inherits from GraphAPI:

  • GraphAPISafeRetry
  • GraphAPIForceRetry

First implementation retries only when we are sure that retry could help and it can't break anything (ie. we can safety retry on GET request, but POST requests are more tricky). Second implementation retries when we think that retry could help, and we have great chance that it will not break anything (but we are not 100% sure).

Do you have any better name than ForceRetry (it suggests that we retry always but it is not true).

@jgorset
Owner

I think this is a great idea, but why don't we introduce a retry keyword argument to get, post and delete rather than another class altogether?

 post = graph.post("me/feed",
    message = "Hello, world!",
    retry = 3
 )
@tomwys

If we want code retry mechanism in main class, we have to choose retry strategy. I'm not sure if choosing single retry strategy is good decision. This is why I proposed to implement many retry strategies.

but. you can implement it in other way:

class GraphAPI:
    def __init__(self, ..., retry_strategy = None):
        ....
        if retry_strategy is None:
            retry_strategy = DefaultRetryStrategy()
        self.retry_strategy = retry_strategy

and then use it in way you proposed.

What do you think about that?

@jgorset
Owner

I may have misunderstood you, but wouldn't a retry keyword argument to get, post and delete effectively facilitate for choosing a retry strategy for each request?

For example, the following two pieces of code would be identical:

graph = GraphAPISafeRetry()

# Make a GET request to the Graph API and retry on errors.
graph.get("me/feed")

# Make a POST request to the Graph API, but don't retry on errors.
graph.post("me/feed", message="Hello, world!")
graph = GraphAPI()

# Make a GET request to the Graph API and retry on errors.
graph.get("me/feed", retry=3)

# Make a POST request to the Graph API, but don't retry on errors.
graph.post("me/feed", message="Hello, world!")

It would quickly get tedious to specify retry on every request, though.

@jgorset
Owner

I like your last example! We should consider facilitating for changing the retry strategy after initializing the class, too:

# Initialize a new Graph API instance configured to retry non-idempotent requests (GET).
graph = GraphAPI(retry = "safe")

# Get my feed and retry on errors.
graph.get("me/feed")

# Post to my feed, but don't retry on errors.
graph.post("me/feed", message = "Hello, world!")

# Change retry strategy to all requests (GET, POST and DELETE).
with graph.settings(retry = "all"):

    # Get my feed and retry on errors.
    graph.get("me/feed")

    # Post to my feed and retry on errors.
    graph.post("me/feed", message = "Hello, world!")
@tomwys

There are more problems with that. The errors may be divide in two categories:

  • http errors
  • Facebook errors

Facebook errors are problem ones. If you have error message from my first post, you should retry. But if you have error like "you have no permission to do that", you should abort after first try. What if you have error that is unrecognized (we haven't added it to our decision array). Should we retry and log the problem, or should we abort and log the problem, or should we do something else?

In my project I would expect following behavior:

  • retry all things you know that is safe to retry
  • abort when you are not sure and log the problem, so I can improve algorithm

But some users may want one of the following behavior (I don't know if someone will):

  • retry only HTTP errors
  • retry all things you can, I don't care (some less skilled programmers may want this)
@tomwys

Changing strategy sounds wrong. Ie. you can raise error, and due to that don't change strategy back. To fix that you should use changing strategy with "with" or "finnaly" statement and things are getting complicated.

Better solution is using something like "replace_strategy" - produces copy of object but with different strategy.

So user can use it in this way:

graph.replace_strategy(SomeStrategy).get('me')
@jgorset
Owner

There are more problems with that. The errors may be divide in two categories:

  • http errors
  • Facebook errors

Facebook errors are problem ones. If you have error message from my first post, you should retry. But if you have error like "you have no permission to do that", you should abort after first try. What if you have error that is unrecognized (we haven't added it to our decision array). Should we retry and log the problem, or should we abort and log the problem, or should we do something else?

I think we will need to set a reasonable default, and I agree that it should be requests that we know are safe to retry.

Changing strategy sounds wrong. Ie. you can raise error, and due to that don't change strategy back. To fix that you should use changing strategy with "with" or "finnaly" statement and things are getting complicated.

Better solution is using something like "replace_strategy" - produces copy of object but with different strategy.

So user can use it in this way:

I don't know about this — I think with is a lot less complicated than replace_strategy.

@jgorset
Owner

BAM! c673457

@jgorset jgorset closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.