Re-thrown exception occurred in the blocking service call (#21573)

* Rethrown exception occurred in the actual service call

* Fix lint and test

homeassistant/components/websocket_api/commands.py

@@ -3,7 +3,8 @@ import voluptuous as vol
 
 from homeassistant.const import MATCH_ALL, EVENT_TIME_CHANGED
 from homeassistant.core import callback, DOMAIN as HASS_DOMAIN
-from homeassistant.exceptions import Unauthorized, ServiceNotFound
+from homeassistant.exceptions import Unauthorized, ServiceNotFound, \
+    HomeAssistantError
 from homeassistant.helpers import config_validation as cv
 from homeassistant.helpers.service import async_get_all_descriptions
 
@@ -149,6 +150,12 @@ async def handle_call_service(hass, connection, msg):
     except ServiceNotFound:
         connection.send_message(messages.error_message(
             msg['id'], const.ERR_NOT_FOUND, 'Service not found.'))
+    except HomeAssistantError as err:
+        connection.send_message(messages.error_message(
+            msg['id'], const.ERR_HOME_ASSISTANT_ERROR, '{}'.format(err)))
+    except Exception as err:  # pylint: disable=broad-except
+        connection.send_message(messages.error_message(
+            msg['id'], const.ERR_UNKNOWN_ERROR, '{}'.format(err)))
 
 
 @callback

homeassistant/components/websocket_api/const.py

@@ -9,6 +9,7 @@ MAX_PENDING_MSG = 512
 ERR_ID_REUSE = 'id_reuse'
 ERR_INVALID_FORMAT = 'invalid_format'
 ERR_NOT_FOUND = 'not_found'
+ERR_HOME_ASSISTANT_ERROR = 'home_assistant_error'
 ERR_UNKNOWN_COMMAND = 'unknown_command'
 ERR_UNKNOWN_ERROR = 'unknown_error'
 ERR_UNAUTHORIZED = 'unauthorized'

homeassistant/helpers/service.py

@@ -272,7 +272,10 @@ async def entity_service_call(hass, platforms, func, call, service_name=''):
     ]
 
     if tasks:
-        await asyncio.wait(tasks)
+        done, pending = await asyncio.wait(tasks)
+        assert not pending
+        for future in done:
+            future.result()  # pop exception if have
 
 
 async def _handle_service_platform_call(func, data, entities, context):
@@ -294,4 +297,7 @@ async def _handle_service_platform_call(func, data, entities, context):
             tasks.append(entity.async_update_ha_state(True))
 
     if tasks:
-        await asyncio.wait(tasks)
+        done, pending = await asyncio.wait(tasks)
+        assert not pending
+        for future in done:
+            future.result()  # pop exception if have

tests/components/deconz/test_climate.py

@@ -1,6 +1,8 @@
 """deCONZ climate platform tests."""
 from unittest.mock import Mock, patch
 
+import asynctest
+
 from homeassistant import config_entries
 from homeassistant.components import deconz
 from homeassistant.helpers.dispatcher import async_dispatcher_send
@@ -43,8 +45,14 @@ ENTRY_CONFIG = {
 async def setup_gateway(hass, data, allow_clip_sensor=True):
     """Load the deCONZ sensor platform."""
     from pydeconz import DeconzSession
-    loop = Mock()
-    session = Mock()
+
+    session = Mock(put=asynctest.CoroutineMock(
+        return_value=Mock(status=200,
+                          json=asynctest.CoroutineMock(),
+                          text=asynctest.CoroutineMock(),
+                          )
+        )
+    )
 
     ENTRY_CONFIG[deconz.const.CONF_ALLOW_CLIP_SENSOR] = allow_clip_sensor
 
@@ -52,7 +60,7 @@ async def setup_gateway(hass, data, allow_clip_sensor=True):
         1, deconz.DOMAIN, 'Mock Title', ENTRY_CONFIG, 'test',
         config_entries.CONN_CLASS_LOCAL_PUSH)
     gateway = deconz.DeconzGateway(hass, config_entry)
-    gateway.api = DeconzSession(loop, session, **config_entry.data)
+    gateway.api = DeconzSession(hass.loop, session, **config_entry.data)
     gateway.api.config = Mock()
     hass.data[deconz.DOMAIN] = gateway
 

tests/components/websocket_api/test_commands.py

@@ -7,6 +7,7 @@ from homeassistant.components.websocket_api.auth import (
     TYPE_AUTH, TYPE_AUTH_OK, TYPE_AUTH_REQUIRED
 )
 from homeassistant.components.websocket_api import const, commands
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.setup import async_setup_component
 
 from tests.common import async_mock_service
@@ -66,6 +67,51 @@ async def test_call_service_not_found(hass, websocket_client):
     assert msg['error']['code'] == const.ERR_NOT_FOUND
 
 
+async def test_call_service_error(hass, websocket_client):
+    """Test call service command with error."""
+    @callback
+    def ha_error_call(_):
+        raise HomeAssistantError('error_message')
+
+    hass.services.async_register('domain_test', 'ha_error', ha_error_call)
+
+    async def unknown_error_call(_):
+        raise ValueError('value_error')
+
+    hass.services.async_register(
+        'domain_test', 'unknown_error', unknown_error_call)
+
+    await websocket_client.send_json({
+        'id': 5,
+        'type': commands.TYPE_CALL_SERVICE,
+        'domain': 'domain_test',
+        'service': 'ha_error',
+    })
+
+    msg = await websocket_client.receive_json()
+    print(msg)
+    assert msg['id'] == 5
+    assert msg['type'] == const.TYPE_RESULT
+    assert msg['success'] is False
+    assert msg['error']['code'] == 'home_assistant_error'
+    assert msg['error']['message'] == 'error_message'
+
+    await websocket_client.send_json({
+        'id': 6,
+        'type': commands.TYPE_CALL_SERVICE,
+        'domain': 'domain_test',
+        'service': 'unknown_error',
+    })
+
+    msg = await websocket_client.receive_json()
+    print(msg)
+    assert msg['id'] == 6
+    assert msg['type'] == const.TYPE_RESULT
+    assert msg['success'] is False
+    assert msg['error']['code'] == 'unknown_error'
+    assert msg['error']['message'] == 'value_error'
+
+
 async def test_subscribe_unsubscribe_events(hass, websocket_client):
     """Test subscribe/unsubscribe events command."""
     init_count = sum(hass.bus.async_listeners().values())

tests/test_core.py

@@ -727,8 +727,7 @@ class TestServiceRegistry(unittest.TestCase):
         """Test registering and calling an async service."""
         calls = []
 
-        @asyncio.coroutine
-        def service_handler(call):
+        async def service_handler(call):
             """Service handler coroutine."""
             calls.append(call)
 
@@ -804,6 +803,45 @@ class TestServiceRegistry(unittest.TestCase):
         self.hass.block_till_done()
         assert len(calls_remove) == 0
 
+    def test_async_service_raise_exception(self):
+        """Test registering and calling an async service raise exception."""
+        async def service_handler(_):
+            """Service handler coroutine."""
+            raise ValueError
+
+        self.services.register(
+            'test_domain', 'register_calls', service_handler)
+        self.hass.block_till_done()
+
+        with pytest.raises(ValueError):
+            assert self.services.call('test_domain', 'REGISTER_CALLS',
+                                      blocking=True)
+            self.hass.block_till_done()
+
+        # Non-blocking service call never throw exception
+        self.services.call('test_domain', 'REGISTER_CALLS', blocking=False)
+        self.hass.block_till_done()
+
+    def test_callback_service_raise_exception(self):
+        """Test registering and calling an callback service raise exception."""
+        @ha.callback
+        def service_handler(_):
+            """Service handler coroutine."""
+            raise ValueError
+
+        self.services.register(
+            'test_domain', 'register_calls', service_handler)
+        self.hass.block_till_done()
+
+        with pytest.raises(ValueError):
+            assert self.services.call('test_domain', 'REGISTER_CALLS',
+                                      blocking=True)
+            self.hass.block_till_done()
+
+        # Non-blocking service call never throw exception
+        self.services.call('test_domain', 'REGISTER_CALLS', blocking=False)
+        self.hass.block_till_done()
+
 
 class TestConfig(unittest.TestCase):
     """Test configuration methods."""