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

Retries don't work using a session adapter with empty prefix ("") #6533

Closed
matthewarmand opened this issue Sep 21, 2023 · 2 comments
Closed

Comments

@matthewarmand
Copy link
Contributor

When using a Requests Session with an HTTPAdapter with urllib3.util.Retry configuration supplied, when the adapter is bound to an empty string prefix (""), the retries don't execute.

Expected Result

This was an attempt to set up a session where every request in the session would have the retries executed, regardless of the prefix. A glance at the Requests Session source code seems to indicate this should work... the empty string can be a key in the OrderedDict storing the Session's adapters, and checking url.startswith("") should return True. However...

Actual Result

The retry configuration is ignored, and you're given a Response object with the erroneous error code instead of a raised requests.exceptions.RetryError. When binding the Adapter to the session with an actual prefix (for example, "https://"), the Retry behavior is observed.

Reproduction Steps

The below script can be run to show the behavior, and below that I've posted the output I receive from it.

from requests import Session
from requests.adapters import HTTPAdapter
from requests.exceptions import RetryError
from urllib3.util import Retry
​
​
class LogRetry(Retry):
    def __init__(self, *args, **kwargs):
        total_retries = kwargs["total"]
        print(f"Retry {total_retries} initiating...")
        super().__init__(*args, **kwargs)
​
​
def test_case(retry_clazz, prefix):
    s = Session()
    s.mount(
        prefix,
        HTTPAdapter(
            max_retries=retry_clazz(
                total=3,
                backoff_factor=0.1,
                status_forcelist=[500],
                allowed_methods={"POST"},
            )
        ),
    )
    try:
        print(f"Trying with retry class {retry_clazz}, prefix: {prefix}")
        s.post("https://httpbin.org/status/500", {})
    except RetryError:
        print("Got the retry error, hooray!")
    else:
        print("Didn't get the retry error, oh no!")
​
​
test_case(Retry, "https://")
test_case(Retry, "")
test_case(LogRetry, "https://")
test_case(LogRetry, "")

Results:

Trying with retry class <class 'urllib3.util.retry.Retry'>, prefix: https://
Got the retry error, hooray!
Trying with retry class <class 'urllib3.util.retry.Retry'>, prefix: 
Didn't get the retry error, oh no!
Retry 3 initiating...
Trying with retry class <class '__main__.LogRetry'>, prefix: https://
Retry 2 initiating...
Retry 1 initiating...
Retry 0 initiating...
Retry -1 initiating...
Got the retry error, hooray!
Retry 3 initiating...
Trying with retry class <class '__main__.LogRetry'>, prefix: 
Didn't get the retry error, oh no!

System Information

$ python -m requests.help
{
  "chardet": {
    "version": "5.2.0"
  },
  "charset_normalizer": {
    "version": null
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "3.4"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.11.5"
  },
  "platform": {
    "release": "6.5.3-arch1-1",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.28.2"
  },
  "system_ssl": {
    "version": "30100030"
  },
  "urllib3": {
    "version": "1.26.15"
  },
  "using_charset_normalizer": false,
  "using_pyopenssl": false
}             
@matthewarmand matthewarmand changed the title Retries don't work a session adapter with empty prefix ("") Retries don't work using a session adapter with empty prefix ("") Sep 21, 2023
@sigmavirus24
Copy link
Contributor

We use the longest match and document that. So if you do not remove the other adapters this doesn't work and it is the intended behavior

@sigmavirus24 sigmavirus24 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2023
@matthewarmand
Copy link
Contributor Author

Ah, you must be referring to the implicit adapters defined on Session instantiation at

self.mount("https://", HTTPAdapter())
. So that means in order to make this work, you'd have to do something like...

s = Session()
s.adapters.clear()
s.mount("", HTTPAdapter(.....))

That makes sense, thanks. This is one area of requests where the documentation could be much clearer. Would it be appropriate to continue extending the Advanced Usage section I added in #6258 (https://requests.readthedocs.io/en/latest/user/advanced/#example-automatic-retries) with some of these additional details/gotchas? Or would it be better to start a different place in the docs for more in-depth documentation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants