Repository: prkumar/uplink (python)

Issue:
Unexpected behavior of async `response_handler` callback
**Describe the bug**

Changes made in #248 break the async `response_handler` callback behavior.

**To Reproduce**

Define asynchronous response handler:

```python
@uplink.response_handler
async def custom_response_handler(response):
    return "result"
```

Define consumer method decorated this way:

```python
class ExampleConsumer(uplink.Consumer):
    @custom_response_handler
    @uplink.get("/")
    def consumer_method(self):
        pass
```

Call the method:

```python
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.


Pull request:
Do not wrap async function with threaded_callback
Async callbacks are being wrapped by ThreadedCoroutine when they shouldn't be. For some context, the purpose of ThreadedCoroutine is to allow sync callbacks with aiohttp client. When the callback is an async function, [this condition](https://github.com/prkumar/uplink/blob/master/uplink/clients/aiohttp_.py#L89) should skip this behavior.

Fixes #256

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index ddc3a03..dd932c4 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -6,6 +6,12 @@ All notable changes to this project will be documented in this file.
 The format is based on `Keep a Changelog`_, and this project adheres to the
 `Semantic Versioning`_ scheme.
 
+Unreleased
+==========
+Fixed
+-----
+- Fix behavior of async ``@response_handler`` with ``AiohttpClient``. (`#256`_)
+
 0.9.6_ - 2022-01-24
 ===================
 Added
diff --git a/uplink/builder.py b/uplink/builder.py
index dc11996..e051586 100644
--- a/uplink/builder.py
+++ b/uplink/builder.py
@@ -7,6 +7,7 @@ from uplink import (
     arguments,
     auth as auth_,
     clients,
+    compat,
     converters as converters_,
     exceptions,
     helpers,
@@ -41,7 +42,11 @@ class RequestPreparer(object):
         return chain
 
     def _wrap_hook(self, func):
-        return functools.partial(func, self._consumer)
+        @compat.wraps(func)
+        def wrapper(*args, **kwargs):
+            return func(self._consumer, *args, **kwargs)
+
+        return wrapper
 
     def apply_hooks(self, execution_builder, chain):
         # TODO:
diff --git a/uplink/clients/aiohttp_.py b/uplink/clients/aiohttp_.py
index cc64976..4d3a3f3 100644
--- a/uplink/clients/aiohttp_.py
+++ b/uplink/clients/aiohttp_.py
@@ -5,6 +5,7 @@ that returns awaitable responses.
 # Standard library imports
 import asyncio
 import collections
+import inspect
 import threading
 from concurrent import futures
 
@@ -86,7 +87,8 @@ class AiohttpClient(interfaces.HttpClientAdapter):
         return self._session
 
     def wrap_callback(self, callback):
-        if not asyncio.iscoroutinefunction(callback):
+        func = inspect.unwrap(callback)
+        if not asyncio.iscoroutinefunction(func):
             callback = self._sync_callback_adapter(callback)
         return callback
 
diff --git a/uplink/compat.py b/uplink/compat.py
index 6b6ac98..6656fb7 100644
--- a/uplink/compat.py
+++ b/uplink/compat.py
@@ -5,3 +5,4 @@ __all__ = ["abc", "reraise"]
 
 abc = six.moves.collections_abc
 reraise = six.reraise
+wraps = six.wraps
diff --git a/uplink/hooks.py b/uplink/hooks.py
index 20d128e..5f588fa 100644
--- a/uplink/hooks.py
+++ b/uplink/hooks.py
@@ -15,6 +15,7 @@ def _wrap_if_necessary(hook, requires_consumer):
 
 
 def _wrap_to_ignore_consumer(hook):
+    @compat.wraps(hook)
     def wrapper(_, *args, **kwargs):
         # Expects that consumer is the first argument
         return hook(*args, **kwargs)


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index a809aa9..ac6c289 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -5,6 +5,7 @@ if sys.version_info.major < 3:
     collect_ignore.extend(
         [
             "unit/test_aiohttp_client.py",
+            "integration/test_handlers_aiohttp.py",
             "integration/test_retry_aiohttp.py",
         ]
     )
diff --git a/tests/integration/test_handlers_aiohttp.py b/tests/integration/test_handlers_aiohttp.py
new file mode 100644
index 0000000..f048b6c
--- /dev/null
+++ b/tests/integration/test_handlers_aiohttp.py
@@ -0,0 +1,48 @@
+# Local imports.
+import uplink
+
+# Third-party imports
+import aiohttp
+import pytest
+from uplink.clients.aiohttp_ import AiohttpClient
+
+# Constants
+BASE_URL = "https://example.com/"
+SIMPLE_RESPONSE = "simple response"
+
+
+@pytest.fixture
+def mock_aiohttp_session(mocker):
+    return mocker.Mock(spec=aiohttp.ClientSession)
+
+
+@uplink.response_handler
+async def simple_async_handler(response):
+    return SIMPLE_RESPONSE
+
+
+class Calendar(uplink.Consumer):
+    @simple_async_handler
+    @uplink.get("todos/{todo_id}")
+    def get_todo(self, todo_id):
+        pass
+
+
+@pytest.mark.asyncio
+async def test_simple_async_handler(mock_aiohttp_session, mock_response):
+    mock_response.status = 200
+
+    async def request(*args, **kwargs):
+        return mock_response
+
+    mock_aiohttp_session.request = request
+
+    calendar = Calendar(
+        base_url=BASE_URL, client=AiohttpClient(mock_aiohttp_session)
+    )
+
+    # Run
+    response = await calendar.get_todo(todo_id=1)
+
+    # Verify
+    assert response == SIMPLE_RESPONSE
diff --git a/tests/unit/test_hooks.py b/tests/unit/test_hooks.py
index fecada0..0fbd793 100644
--- a/tests/unit/test_hooks.py
+++ b/tests/unit/test_hooks.py
@@ -16,7 +16,7 @@ class TestResponseHandler(object):
 
 class TestRequestAuditor(object):
     def test_audit_request(self, mocker):
-        auditor = mocker.stub()
+        auditor = mocker.Mock()
         ra = hooks.RequestAuditor(auditor)
         ra.audit_request("consumer", "request")
         auditor.assert_called_with("request")
@@ -24,7 +24,7 @@ class TestRequestAuditor(object):
 
 class TestExceptionHandler(object):
     def test_handle_exception_masked(self, mocker):
-        handler = mocker.stub()
+        handler = mocker.Mock()
         eh = hooks.ExceptionHandler(handler)
         eh.handle_exception("consumer", "exc_type", "exc_val", "exc_tb")
         handler.assert_called_with("exc_type", "exc_val", "exc_tb")
@@ -45,8 +45,8 @@ class TestTransactionHookChain(object):
 
     def test_delegate_handle_response_multiple(self, mocker):
         # Include one hook that can't handle responses
-        mock_response_handler = mocker.stub()
-        mock_request_auditor = mocker.stub()
+        mock_response_handler = mocker.Mock()
+        mock_request_auditor = mocker.Mock()
 
         chain = hooks.TransactionHookChain(
             hooks.RequestAuditor(mock_request_auditor),