8000 Improve async generic camera's error handling by kellerza · Pull Request #4316 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Improve async generic camera's error handling #4316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Nov 11, 2016
Merged

Improve async generic camera's error handling #4316

merged 3 commits into from
Nov 11, 2016

Conversation

kellerza
Copy link
Member
@kellerza kellerza commented Nov 8, 2016

Description:
Handle connection errors for generic camera in the async version similar to the non-async version (see line 103)

Logs currently fills with the following error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/site-packages/aiohttp/server.py", line 261, in start
    yield from self.handle_request(message, payload)
  File "/usr/local/lib/python3.5/site-packages/aiohttp/web.py", line 88, in handle_request
    resp = yield from handler(request)
  File "/usr/src/app/homeassistant/components/http.py", line 495, in handle
    result = yield from result
  File "/usr/src/app/homeassistant/components/camera/__init__.py", line 187, in get
    response = yield from self.handle(request, camera)
  File "/usr/src/app/homeassistant/components/camera/__init__.py", line 205, in handle
    image = yield from camera.async_camera_image()
  File "/usr/src/app/homeassistant/components/camera/generic.py", line 114, in async_camera_image
    auth=self._auth
  File "/usr/local/lib/python3.5/site-packages/aiohttp/client.py", line 555, in __iter__
    resp = yield from self._coro
  File "/usr/local/lib/python3.5/site-packages/aiohttp/client.py", line 198, in _request
    conn = yield from self._connector.connect(req)
  File "/usr/local/lib/python3.5/site-packages/aiohttp/connector.py", line 314, in connect
    .format(key, exc.strerror)) from exc
aiohttp.errors.ClientOSError: [Errno 113] Cannot connect to host 192.168.88.90:80 ssl:False [Can not connect to 192.168.88.90:80 [No route to host]]

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

camera:
  - platform: generic
    still_image_url: http://192.168.88.90/cgi-bin/guest/Video.cgi?media=JPEG
    name: Gate

Checklist:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@mention-bot
Copy link

@kellerza, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @pvizeli to be potential reviewers.

pvizeli
pvizeli reviewed Nov 8, 2016
View reviewed changes
Copy link
Member
@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use aiohttp.errors.ClientError to handle more exception.

@kellerza
Copy link
Member Author
kellerza commented Nov 8, 2016

@pvizeli is #4294 related?

@mezz64
Copy link
Contributor
mezz64 commented Nov 8, 2016

I think it is related, but in my case since aiohttp.errors.ServerDisconnectedError comes before aiohttp.errors.ClientError i'm not sure this change would catch it.

@kellerza
Copy link
Member Author
kellerza commented Nov 8, 2016

Let me add DisconnectedError as well, see here

@mezz64
Copy link
Contributor
mezz64 commented Nov 8, 2016

Testing this change out now. I don't see any reason why it wouldn't fix the issue though since DisconnectedError is now included.

@pvizeli
Copy link
Member
pvizeli commented Nov 8, 2016 •
edited
Loading

DisconnectedError is not used for client request stuff. Only in for server stuff...

@balloob
Copy link
Member
balloob commented Nov 9, 2016

@pvizeli is right DisconnectedError is the base exception for both server and client. Use ClientDisconnectedError instead.

@pvizeli
Copy link
Member
pvizeli commented Nov 9, 2016

why not in this way: #4325

@kellerza
Copy link
Member Author
kellerza commented Nov 9, 2016

@pvizeli I prefer to show the reason why it failed and not hiding too much by only saying "error on URL x"

Also changed the yr.no handling to be similar (the current implementation is most likely not working)

@mezz64
Copy link
Contributor
mezz64 commented Nov 10, 2016

In my testing with the latest changes I still see the errors described in #4294. If this isn't the appropriate place to catch the aiohttp.errors.ServerDisconnectedError then where should it be handled?

@pvizeli
Copy link
Member
pvizeli commented Nov 10, 2016

You are sure, that you test with this branch?

@mezz64
Copy link
Contributor
mezz64 commented Nov 11, 2016

Yes. I double checked and have the altered lines in my generic camera file. The thing is I'm having trouble recreating the exact error I get with when using the wunderground url listed in my issue. I had originally thought it was a result of the url not being available, but that doesn't reflect in my tests. It seems the wunderground server is returning a different error in the cases when the exception pops up and I haven't been able to catch it yet.

In any case, the changes proposed are a step forward and successfully catch other connection issues. It may be best to merge this in and keep issue #4294 open until I'm able to get a better handle on what wunderground is actually doing when the errors get logged.

@balloob balloob added this to the 0.32.3 milestone Nov 11, 2016
@balloob balloob merged commit 9bb94a4 into home-assistant:dev Nov 11, 2016
@balloob balloob mentioned this pull request Nov 11, 2016
Closed
balloob pushed a commit that referenced this pull request Nov 11, 2016
@kellerza @balloob
Improve async generic camera's error handling (#4316)
2c39c39 
* Handle errors

* Feedback

* DisconnectedError
@kellerza kellerza deleted the camera branch November 21, 2016 04:00
@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pvizeli pvizeli pvizeli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.32.3
Development

Successfully merging this pull request may close these issues.
5 participants
@kellerza @mention-bot @mezz64 @pvizeli @balloob
0