Unexpected behavior of async `response_handler` callback

[returnnullptr]

Describe the bug

Changes made in #248 break the async response_handler callback behavior.

To Reproduce

Define asynchronous response handler:

@uplink.response_handler
async def custom_response_handler(response):
    return "result"

Define consumer method decorated this way:

class ExampleConsumer(uplink.Consumer):
    @custom_response_handler
    @uplink.get("/")
    def consumer_method(self):
        pass

Call the method:

async def main(base_url: str):
    async with aiohttp.ClientSession() as session:
        example_consumer = ExampleConsumer(
            base_url=base_url,
            client=uplink.AiohttpClient(session),
        )

        # expected: "result", actual: <coroutine object custom_response_handler at 0x7f7c7ccbc8c0>
        # In the `0.9.5`, asynchronous callbacks worked as expected
        result = await example_consumer.consumer_method()

        # double await works, but this is unexpected
        result = await (await example_consumer.consumer_method())

Since the 0.9.6, awaited method call is a coroutine.

Expected behavior

A consumer method decorated this way returns a result using a single await.

Additional context

The aiohttp client is used.

[prkumar]

Hey, @returnnullptr - thanks for reporting! I think you caught something deeper. #258 should fix this.

[HarvsG]

This also seems to have broken my app. I find that objects I await return a co-routine object and have to be awaited again....

[prkumar]

Hey, @HarvsG - Got it. The fix will be part of v0.9.7, which I'll be releasing this week. ETA is by March 11th

[HarvsG]

Great, my app actually targets uplink 0.9.4 but is is used within Homeassisstant which seems to be forcing it to use a newer version with the bug.

Edit: In the meantime I have fixed the issue by ssh-ing into my machine, then running pip install uplink==0.9.5 in the relevant container.

[prkumar]

v0.9.7 is live now and includes the fix for this issue

[HarvsG]

hmm, still getting the same error with 0.9.7

[prkumar]

@HarvsG - Interesting.. I might have missed something, or you might be running into a similar issue with a different cause. Could you check out this test case and let me know if it captures your usage? (i.e., invoking a method wrapped by response handler with an async http client)

[jamur2]

This might be a different issue altogether, in which case I can file a separate ticket, but we've found a similar issue with the usage of the aiohttp client and uplink >= 0.9.6.

In our system, we enqueue many (tens of thousands) requests, and execute them with a concurrency of, say, 100. For example:

 client = AiohttpClient(aiohttp.ClientSession(connector=aiohttp.TCPConnector(limit=100),
                                              timeout=aiohttp.ClientTimeout(total=600)))

In short, we've noticed that when using an aiohttp client in conjunction with the @retry decorator, after the first retry event, performance slows to a trickle (multiple minutes between requests), and eventually seemingly stops.

We've added logging and found that these retried requests don't spend that time waiting for a connection or anything; once the on_request_start event is fired for the retried request, they immediately get a connection and execute the request, it just takes them forever to get requeued and for the on_request_start event to fire.

When using the aiohttp client with uplink 0.9.5, the retry logic works as expected and is fully performant even in the case of occasional retries due to server/network errors.

I'm not sure if this is any relation to the response_handler callback issue mentioned here, but the timing and relation to aiohttp made us think this could be related.