Skip to content

Log exceptions thrown by signal callbacks#20015

Merged
balloob merged 10 commits intohome-assistant:devfrom
emontnemery:dispatcher_log_exceptions
Jan 17, 2019
Merged

Log exceptions thrown by signal callbacks#20015
balloob merged 10 commits intohome-assistant:devfrom
emontnemery:dispatcher_log_exceptions

Conversation

@emontnemery
Copy link
Copy Markdown
Contributor

@emontnemery emontnemery commented Jan 12, 2019

Description:

Improve logging of exceptions thrown by MQTT message callbacks as discussed in #15446

Example of improved logging:

ERROR:homeassistant.components.lock.mqtt:Exception in async_discover when dispatching 'mqtt_discovery_new_lock_mqtt': '{'name': 'Beer', 'platform': 'mqtt', 'state_topic': 'homeassistant/lock/bla/state', 'discovery_hash': ('lock', 'bla')}'
Traceback (most recent call last):
  File "/mnt/d/development/github/home-assistant_fork/homeassistant/components/lock/mqtt.py", line 63, in async_discover
    config = PLATFORM_SCHEMA(discovery_payload)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 267, in __call__
    return self._compiled([], data)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 589, in validate_dict
    return base_validate(path, iteritems(data), out)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 427, in validate_mapping
    raise er.MultipleInvalid(errors)
voluptuous.error.MultipleInvalid: required key not provided @ data['command_topic']

Example of improved logging if not skipping the wrapper:

ERROR:homeassistant.components.lock.mqtt:Exception in async_discover when dispatching 'mqtt_discovery_new_lock_mqtt': '{'name': 'Beer', 'platform': 'mqtt', 'state_topic': 'homeassistant/lock/bla/state', 'discovery_hash': ('lock', 'bla')}'
Traceback (most recent call last):
  File "/mnt/d/development/github/home-assistant_fork/homeassistant/helpers/dispatcher.py", line 51, in wrapper
    await func(*args)
  File "/mnt/d/development/github/home-assistant_fork/homeassistant/components/lock/mqtt.py", line 63, in async_discover
    config = PLATFORM_SCHEMA(discovery_payload)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 267, in __call__
    return self._compiled([], data)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 589, in validate_dict
    return base_validate(path, iteritems(data), out)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 427, in validate_mapping
    raise er.MultipleInvalid(errors)
voluptuous.error.MultipleInvalid: required key not provided @ data['command_topic']

Example of old error message:

ERROR:homeassistant.core:Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/mnt/d/development/github/home-assistant_fork/homeassistant/components/lock/mqtt.py", line 63, in async_discover
    config = PLATFORM_SCHEMA(discovery_payload)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 267, in __call__
    return self._compiled([], data)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 589, in validate_dict
    return base_validate(path, iteritems(data), out)
  File "/mnt/d/development/github/home-assistant_fork/lib/python3.6/site-packages/voluptuous/schema_builder.py", line 427, in validate_mapping
    raise er.MultipleInvalid(errors)
voluptuous.error.MultipleInvalid: required key not provided @ data['command_topic']

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@emontnemery emontnemery requested a review from a team as a code owner January 12, 2019 11:02
@ghost ghost assigned emontnemery Jan 12, 2019
@ghost ghost added the in progress label Jan 12, 2019
@emontnemery
Copy link
Copy Markdown
Contributor Author

Are tests needed for this change?

@emontnemery emontnemery force-pushed the dispatcher_log_exceptions branch from ed80b2e to f51a24b Compare January 12, 2019 12:33
@emontnemery emontnemery force-pushed the dispatcher_log_exceptions branch from 1a76ad8 to ff9dce8 Compare January 17, 2019 16:04
@balloob balloob merged commit 6800871 into home-assistant:dev Jan 17, 2019
@ghost ghost removed the in progress label Jan 17, 2019
@emontnemery emontnemery deleted the dispatcher_log_exceptions branch January 19, 2019 10:27
@andrewsayre
Copy link
Copy Markdown
Member

andrewsayre commented Jan 20, 2019

@emontnemery this change introduces a bug when the target is a functools.partial. We need to check that the target is a partial, then inspect the contained function to know whether it should be a coroutine or not. See #20119 where this was added to async_add_job, which the dispatcher calls.

Edit: I'll submit a PR to address this -- just running local tests now before I submit.

@balloob balloob mentioned this pull request Feb 6, 2019
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Feb 12, 2019
* Log exceptions thrown by signal callbacks

* Fix unsub

* Simplify traceback print

* Typing

* Add test

* lint

* Review comments

* Rework MQTT test case

* Fix bad merge

* Fix bad merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants