search

goedh452 / homebridge-http-sprinkler

Homebridge http controlled sprinkler
Apache License 2.0
12 stars 6 forks source link

Homebridge crashes on uncaught exception. #1

Closed phenotypic closed 6 years ago

phenotypic commented 6 years ago

Hey there,

Before I start, I'd like to thank you for making this plugin as it's exactly what I've been looking for! I.e. a http homebridge plugin which supports the faucet/sprinkler accessory type. It usually works exactly as I'd wish. However, the problem comes when the device meant to be receiving the http request is unavailable either because it is down or unreachable.

In this case, it quickly causes homebridge to crash without any other warnings. However, with homebridge-zp installed you can see what the issue is due to the fact that homebridge-zp installs a handler for otherwise uncaught exceptions. Now, when the request isn't received by the target, I get the following before crashing:

[2018-8-8 22:41:22] [Misters] Setting power state to on
[2018-8-8 22:41:26] [ZP] uncaught exception
[2018-8-8 22:41:26] [ZP] Error: connect EHOSTUNREACH 192.168.1.146:80
    at doRequest (/usr/local/lib/node_modules/homebridge-http-sprinkler/node_modules/sync-request/index.js:31:11)
    at HttpSprinkler.setPowerState (/usr/local/lib/node_modules/homebridge-http-sprinkler/index.js:190:13)
    at emitMany (events.js:147:13)
    at Characteristic.Active.emit (events.js:224:7)
    at Characteristic.Active.Characteristic.setValue (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/lib/Characteristic.js:320:10)
    at Bridge.<anonymous> (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/lib/Accessory.js:871:22)
    at Array.forEach (<anonymous>)
    at Bridge.Accessory._handleSetCharacteristics (/usr/local/lib/node_modules/homebridge/node_modules/hap-nodejs/lib/Accessory.js:811:8)
    at emitMany (events.js:147:13)
    at HAPServer.emit (events.js:224:7)
[2018-8-8 22:41:26] Got SIGTERM, shutting down Homebridge...
[2018-8-8 22:41:26] [ZP] cleaning up...
[2018-8-8 22:41:31] [ZP] exit

It lists the full stack trace of the exception, which is raised by the sync-request module that's included by homebridge-http-sprinkler:

[2018-8-8 22:41:26] [ZP] Error: connect EHOSTUNREACH 192.168.1.146:80
   at doRequest (/usr/local/lib/node_modules/homebridge-http-sprinkler/node_modules/sync-request/index.js:31:11)

In this case, homebridge-http-sprinkler fails to handle this exception:

at HttpSprinkler.setPowerState (/usr/local/lib/node_modules/homebridge-http-sprinkler/index.js:190:13)

My question is: would you be able to update homebridge-http-sprinkler to fix this problem? I am aware that homebridge-http uses the asynchronous library request which would omit this issue.

Thank you in advance for any help,

Kind regards, Tom

goedh452 commented 6 years ago

Hi Tom,

I must admit that I'm by no means a great coder. Ik copy pasted this plugin together from different other plugins and the help of forums. But that doesn't mean I'm not willing to give it a try :-).

I'll look into it when I've some time to spend. In the mean time, should you have any idea how to catch this exception, I'm open to suggestions!

Regards, Bouke

phenotypic commented 6 years ago

Hey Bouke,

Thanks for your reply! I really appreciate you taking the time to consider my issue. Unfortunately, I too, am admittedly not an amazing coder especially in languages other than bash. However, I do have an admittedly crude solution which may work if you could figure out how to implement it into your current plugin:

As I said, I have very little programming experience outside of bash so am unsure how (if you even can) implement the following: As I'm sure you're aware, you can use the command curl to simply do a GET request; however, a straightforward curl http://yourip/turnon would hang in the same way that you're current plugin hangs if there is no response. However, there is a flag in curl called -m which I believe stands for "Maximum Time". Implementing it like this: curl -m 5 http://yourip/turnon means that if there is no response after 5 seconds, you will simply get an error like curl: (52) Empty reply from server rather than it hanging and causing a crash. This enables you to at least handle the request i.e. simply print out something into homebridge like Didn't hear back from device, carrying on... when you receive the error message from curl rather than causing a crash.

Now, I am unaware as to how exactly you could implement this into your plugin (if you even want to); but, I assume you are using some sort of function to make a GET request. I thought maybe it would be possible to instead use some sort of function to run shell/bash commands where you simply substitute in the URL from curl -m 5 http://yourip/turnon with the one that the user specifies.

Again, it is completely up to you how you want to approach the problem as you may not wish to diverge from your current method. Either way, I really do hope that you are able to find some sort of solution when you are able to!

Thanks once again,

Kind regards, Tom

goedh452 commented 6 years ago

Hi Tom,

I seemed to have fixed the issue. I noticed that the polling mechanism did have an event handler for when the host is unreachable and simply copy-pasted the code to the set function. With some adjustments it won't crash Homebridge anymore, but writes an error to the log. After about 30 seconds the tile in the home app shows the ! indicating there is a problem.

Install the updated plugin and let me now it it works for you too!

phenotypic commented 6 years ago

Hey Bouke,

Thank you so much! Works absolutely perfectly now! I'm so glad that you were able to find a solution to the problem. As you suggested, now when the request is not handled, homebridge simply displays:

HTTP set status function failed connect ETIMEDOUT <URL>

as opposed to simply crashing; which is great. Indeed, the Home app does later show the ! icon after around 30 seconds.

Thank you once again for all your help! I'm very happy now.

Kind regards, Tom

P.S. Not at all an issue to do with your plugin, but: Are you aware of how to integrate faucets/sprinkler accessories into Home Automations such as when it reaches a certain time for example? Because currently, any accessories of this type don't even appear in the Automation section.

goedh452 commented 6 years ago

You're very welcome.

About the P.S.: I think this is a shortcoming of the home app. I do not see the sprinkler in automations as well, but also the option in the details page of the sprinkler "Include in status en notification" is missing. No idea why. I'm running iOS12 beta and this has the same behavior. Or maybe it is a characteristic I can add to the plugin. I was hoping to check that somewhere in the coming days.

phenotypic commented 6 years ago

I noticed the issue with the missing "Include in status in notification" option too. Vey strange indeed! I would've at least expected Apple to solve the issue in IOS 12 if it is an issue on their part as there are now a number of consumer, Home compatible, sprinkler devices available; so it is strange to see no mention of the missing Automation option in the Home app anywhere online. If it is a characteristic you could add to the plugin, it would be great if you could work out some way to enable it in the plugin! Thank you!

EDIT: Might this link here help you for any characteristics?

phenotypic commented 6 years ago

Hi Bouke,

Sorry to pester you again. The plugin has been working perfectly well for me without any crashing problems, thank you!

I just wanted to check up with you whether or not you have found a way of enabling the missing "Include in status in notification" option as well as adding the option to include the accessory in Automations/Scenes?

Thanks once again for your work, Kind regards, Tom

goedh452 commented 6 years ago

Hi Tom,

I've checked it, but didn't solve it. I reported it as a bug with Apple. I checked your link. There doesn't seem to be a characteristic for the notifications.

I'm experiencing bigger issues with the plugin I wrote in iOS 12 Beta. Even since beta release 6 the switch wil activate but keeps "hanging" in status "starting" (I don't know which English word is used in the app since my phone is set to Dutch). This eventually makes the switch unusable.

Do you have the same problem with the plugin or is it still working fine for you?

EDIT: fixed the issue with the tile hanging in "inactive".

Bouke

phenotypic commented 6 years ago

Thanks for your reply!

It is very strange that it seems there isn't currently any support for Automations/Scenes with this type of accessory. I really appreciate you looking into the issue further and filing a bug report with Apple; hopefully it's an issue which can be quickly resolved on their part in a future iOS release.

Unfortunately, I am not currently enrolled in the iOS 12 Beta on my iPhone so haven't been able to do much in the way of testing homebridge functionality with the new releases. However, I must admit that whilst your plugin has been working very well for much of the time, there have been occasions where it is stuck in the "Updating" status which is what I believe you were referencing in the bug you mentioned. However, admittedly, the issues I have been experiencing in IOS 11 (ignoring the Automations/Scenes issue) have been very minimal; whilst I did experience the hanging for a short period of time, it was not too much of an issue and I am grateful for the work you have put in trying to iron out this "bug" along with any others.

Hopefully the strange situation with the missing Automations/Scenes options will be resolved soon.

Best, Tom