fix(client): assure error not called on done (#3934)

* fix(client): assure error not called on done

* fix(client): refine flow name

* feat(client): add more tests

* fix(client): move assert out

jina/clients/helper.py

@@ -79,7 +79,7 @@ def callback_exec(
     """
     if on_error and response.status.code >= jina_pb2.StatusProto.ERROR:
         _safe_callback(on_error, continue_on_error, logger)(response)
-    elif on_done:
+    elif on_done and response.status.code == jina_pb2.StatusProto.SUCCESS:
         _safe_callback(on_done, continue_on_error, logger)(response)
     if on_always:
         _safe_callback(on_always, continue_on_error, logger)(response)

tests/unit/clients/test_helper.py

@@ -1,6 +1,6 @@
 import aiohttp
 import pytest
-from jina import Flow
+from jina import Flow, Executor, requests
 from jina.logging.logger import JinaLogger
 from jina.types.request import Request
 from jina.clients.request.helper import _new_data_request
@@ -9,6 +9,17 @@
 logger = JinaLogger('clientlet')
 
 
+class ClientTestExecutor(Executor):
+    @requests
+    def error(self, **kwargs):
+        raise NotImplementedError
+
+
+@pytest.fixture
+def flow_with_exception_request():
+    return Flow().add(uses=ClientTestExecutor).add()
+
+
 @pytest.mark.asyncio
 async def test_http_clientlet():
     with Flow(port_expose=12345, protocol='http').add():
@@ -30,3 +41,27 @@ async def test_websocket_clientlet():
             url='ws://localhost:12345', logger=logger
         ) as iolet:
             pass
+
+
+def test_client_behaviour(flow_with_exception_request, mocker):
+    on_done_mock = mocker.Mock()
+    on_always_mock = mocker.Mock()
+    on_error_mock = None
+
+    with flow_with_exception_request as f:
+        f.post(
+            '', on_done=on_done_mock, on_error=on_error_mock, on_always=on_always_mock
+        )
+    on_always_mock.assert_called_once()
+    on_done_mock.assert_not_called()
+
+    on_error_mock = mocker.Mock()
+    on_done_mock = mocker.Mock()
+    on_always_mock = mocker.Mock()
+    with flow_with_exception_request as f:
+        f.post(
+            '', on_done=on_done_mock, on_error=on_error_mock, on_always=on_always_mock
+        )
+    on_always_mock.assert_called_once()
+    on_done_mock.assert_not_called()
+    on_error_mock.assert_called_once()