Skip to content
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.

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

Fix #3073 - Remote motionEye Error #3079

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Matthew1471
Copy link

If a remote motionEye is unavailable an exception was not handled.. this was due to the entirely reasonable expectation raise_error=False would not raise an error. This is however not the case as documented in the tornado.httpclient documentation:

The raise_error=False argument only affects the HTTPError raised when a non-200 response code is used, instead of suppressing all errors.

Not entirely sure the other error 2 paths work either (i.e. response.error appears to expect an exception not a string).. but this commit fixes more than we started with and will fix more than it breaks 😄.

If a remote motionEye is unavailable an exception was not handled.. this was due to the entirely reasonable expectation `raise_error=False` would not raise an error. This is however not the case as documented in the [tornado.httpclient](https://www.tornadoweb.org/en/stable/httpclient.html#tornado.httpclient.AsyncHTTPClient.fetch) documentation:

```
The raise_error=False argument only affects the HTTPError raised when a non-200 response code is used, instead of suppressing all errors.
```

Not entirely sure the other error 2 paths work either (i.e. `response.error` appears to expect an exception not a string).. but this commit fixes more than we started with and will fix more than it breaks 😄.
@zagrim
Copy link
Collaborator

zagrim commented Dec 9, 2024

It would be really nice if someone with existing setup having a remote ME would test this out.

Myself I don't have such a setup, so it would be a bit of work to try it out 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants