Pass raise_for_status param through _client.request function

[jeffreylin]

Summary

Expected: Working Behavior

The raise_for_status param in aiohttp can be set on both aiohttp.ClientSession as well as individual HTTP method calls like session.post().

If you have raise_for_status set on both the ClientSession and the session.post() method, then according to the aiohttp docs, the session.post() method ought to take precedent, and this is what we see when we use aiohttp directly without aiohttp-retry.

Observed: Broken Behavior

When we pass in our own client_session like so:

async with aiohttp.ClientSession(raise_for_status=session_raise) as client_session:
    async with RetryClient(client_session=client_session) as session:
        try:
            async with session.post("https://httpstat.us/500", raise_for_status=request_raise) as response:
              ...

The retry client ignores the raise_for_status param from the session.post() and doesn't pass it to the inner aiohttp.ClientSession as expected. As a result, we get Exceptions thrown where we expect responses to return. See the tests section below.

Fix

The fix is to pass raise_for_status through to the _request_func method called from the original aiohttp.ClientSession. That way the aiohttp.ClientSession can respect the raise_for_status on the session.post()

Tests

Expected Behavior

request_raise should override session_raise, so anytime request_raise is True, we should expect an Error thrown, and anytime request_raise is False, we should expect a normal status+text.

>>> asyncio.run(main())

Testing: session_raise=True, request_raise=True
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=True, request_raise=False
Response status: 500
Response text: 500 Internal Server Error

Testing: session_raise=False, request_raise=True
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=False, request_raise=False
Response status: 500
Response text: 500 Internal Server Error
>>>

No RetrySession -> Expected Behavior

Test

import aiohttp
import asyncio

async def test_500(session_raise, request_raise):
    print(f"\nTesting: session_raise={session_raise}, request_raise={request_raise}")
    
    async with aiohttp.ClientSession(raise_for_status=session_raise) as session:
        try:
            async with session.post("https://httpstat.us/500", raise_for_status=request_raise) as response:
                print(f"Response status: {response.status}")
                text = await response.text()
                print(f"Response text: {text}")
        except aiohttp.ClientResponseError as e:
            print(f"❌ ClientResponseError: {e.status} - {e}")
        except aiohttp.ClientError as e:
            print(f"❌ ClientError: {e}")
        except Exception as e:
            print(f"❌ Other Exception: {e}")

async def main():
    # Test all four combinations
    await test_500(session_raise=True, request_raise=True)
    await test_500(session_raise=True, request_raise=False)
    await test_500(session_raise=False, request_raise=True)
    await test_500(session_raise=False, request_raise=False)

asyncio.run(main())

Results

>>> asyncio.run(main())

Testing: session_raise=True, request_raise=True
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=True, request_raise=False
Response status: 500
Response text: 500 Internal Server Error

Testing: session_raise=False, request_raise=True
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=False, request_raise=False
Response status: 500
Response text: 500 Internal Server Error
>>>

RetrySession -> Broken Behavior

Test

import aiohttp
import asyncio
from aiohttp_retry import RetryClient, ExponentialRetry

async def test_500(session_raise, request_raise, retry_attempts=3):
    print(f"\nTesting: session_raise={session_raise}, request_raise={request_raise}, retries={retry_attempts}")
    retry_options = ExponentialRetry(
        attempts=retry_attempts,
        statuses={408, 429, 502, 503, 504},  # Retry these status codes
        exceptions={
            aiohttp.ServerDisconnectedError,
            aiohttp.ClientOSError,
            aiohttp.ClientResponseError,
            asyncio.TimeoutError,
        }
    )
    async with aiohttp.ClientSession(raise_for_status=session_raise) as client_session:
        async with RetryClient(client_session=client_session, retry_options=retry_options) as session:
            try:
                async with session.post("https://httpstat.us/500", raise_for_status=request_raise) as response:
                    print(f"Response status: {response.status}")
                    text = await response.text()
                    print(f"Response text: {text}")
            except aiohttp.ClientResponseError as e:
                print(f"❌ ClientResponseError: {e.status} - {e}")
            except aiohttp.ClientError as e:
                print(f"❌ ClientError: {e}")
            except Exception as e:
                print(f"❌ Other Exception: {e}")

async def main():
    retry_attempts = 3  # Adjust retry count if needed
    await test_500(session_raise=True, request_raise=True, retry_attempts=retry_attempts)
    await test_500(session_raise=True, request_raise=False, retry_attempts=retry_attempts)
    await test_500(session_raise=False, request_raise=True, retry_attempts=retry_attempts)
    await test_500(session_raise=False, request_raise=False, retry_attempts=retry_attempts)

asyncio.run(main())

Results

>>> asyncio.run(main())

Testing: session_raise=True, request_raise=True, retries=3
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=True, request_raise=False, retries=3
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=False, request_raise=True, retries=3
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=False, request_raise=False, retries=3
Response status: 500
Response text: 500 Internal Server Error
>>>

Caution

Testing: session_raise=True, request_raise=False, retries=3 should not throw an error! The request_raise should take precedence over the session_raise

RetrySession with this Fix Branch -> Expected Behavior

Test

(Same as before)

Results

>>> asyncio.run(main())

Testing: session_raise=True, request_raise=True
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=True, request_raise=False
Response status: 500
Response text: 500 Internal Server Error

Testing: session_raise=False, request_raise=True
❌ ClientResponseError: 500 - 500, message='Internal Server Error', url='https://httpstat.us/500'

Testing: session_raise=False, request_raise=False
Response status: 500
Response text: 500 Internal Server Error
>>>