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

Raise the real error if max retries failed. #4389

Closed
orzilca opened this issue Nov 19, 2017 · 4 comments
Closed

Raise the real error if max retries failed. #4389

orzilca opened this issue Nov 19, 2017 · 4 comments

Comments

@orzilca
Copy link

orzilca commented Nov 19, 2017

When using Retry(), if max retries is reached an error is thrown "max retries".

Is it possible to see the real API error? the real HTTP response instead?

Expected Result

The real error/http response. (ResponseError in the example below)

Actual Result

max retries error, not logging correctly on Sentry.

MaxRetryError(\"HTTPSConnectionPool(host='***', port=443): Max retries exceeded with url: *** (Caused by ResponseError('too many 401 error responses',))\",) is not JSON serializable

Reproduction Steps

import requests
import urllib
import logging
import json
from requests.packages.urllib3.util.retry import Retry

session = requests.Session()
        retries = Retry(
            total=3,
            backoff_factor=0.5,
            method_whitelist=frozenset(['HEAD', 'TRACE', 'GET', 'PUT', 'OPTIONS', 'DELETE', 'POST']),
            status_forcelist=[401, 403, 404, 500, 502, 503, 504]
        )
        session.mount("http://", requests.adapters.HTTPAdapter(max_retries=retries))
        session.mount("https://", requests.adapters.HTTPAdapter(max_retries=retries))

        if method == 'GET':
            if params:
                endpoint = '%s?%s' % (endpoint, urllib.urlencode(params))
            response = session.get(endpoint, headers=headers)
        elif method == 'POST':
            response = session.post(endpoint, data=params, headers=headers)
        elif method == 'POST/JSON':
            response = session.post(endpoint, json=params, headers=headers)
        elif method == 'PUT/JSON':
            response = session.put(endpoint, json=params, headers=headers)
        else:
            raise Exception("Invalid method.")

System Information

$ python -m requests.help
Python 2.7.5 (default, Nov  6 2016, 00:28:07) 
[GCC 4.8.5 20150623 (Red Hat 4.8.5-11)] on linux2
Requests version 2.13
urllib3 version 1.20
@sigmavirus24
Copy link
Contributor

Which error is the real one? What happens if for each of your three retries, we get a different error? Let's say, first we get a ConnectionTimeout. Then we receive a 500 and then a 403? Which one do you want us to tell you about?

@orzilca
Copy link
Author

orzilca commented Nov 20, 2017

Thats a good question @sigmavirus24

Basically I wish to know to real error from the API I tried to call so it'll be easier to debug using tools like Sentry.

Currently it logges under MaxRetries error which forces me to try and reconstruct, and thats not always possible.

@sigmavirus24
Copy link
Contributor

Basically I wish to know to real error from the API

I think you may want a different way of retrying errors then. In fact, I think you may want to have a separate abstraction such that you can attempt a request, send the error to sentry, and then repeat up until you hit your limit. That would give you the "real error" on each attempt.

Right now, the exception we're raising really makes the most sense (MaxRetries) because we've reached our Retry limit (specified by you) and there's nothing more for us to do. Browsers have similar behaviour. If a website redirects too many times or fails to load too many times, Browsers will tell you as much.

@sigmavirus24
Copy link
Contributor

Right, so I'm closing this now. I don't think that Requests or urllib3 should try to keep track of each error that led to a retry. If we did that, we could start leaking resources or even other sensitive information that users may not want stored. Perhaps, urllib3 would want to consider allowing the user to specify some kind of callback for each retry (which a user could then pass a logger to or some other function that posts to an error handling service) but that belongs there, not here :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants