search

KraigM / homebridge-wink

Wink hub plugin for HomeBridge
54 stars 37 forks source link

homebridge-wink causes homebridge to crash if wink API is unavailable. #16

Closed msroest closed 8 years ago

msroest commented 8 years ago

[Wink] Refreshing Wink Data response in http: <!DOCTYPE html>

Application Error

SyntaxError: Unexpected token < at Object.parse (native) at IncomingMessage. (/usr/lib/node_modules/homebridge-wink/node_modules/wink-js/index.js:67:19) at emitNone (events.js:72:20) at IncomingMessage.emit (events.js:166:7) at endReadableNT (_stream_readable.js:905:12) at nextTickCallbackWith2Args (node.js:478:9) at process._tickCallback (node.js:392:17) raw message <!DOCTYPE html>

Application Error

/usr/lib/node_modules/homebridge-wink/node_modules/wink-js/index.js:223 for( var dataIndex in data.data ) { ^

TypeError: Cannot read property 'data' of undefined at /usr/lib/node_modules/homebridge-wink/node_modules/wink-js/index.js:223:41 at IncomingMessage. (/usr/lib/node_modules/homebridge-wink/node_modules/wink-js/index.js:74:7) at emitNone (events.js:72:20) at IncomingMessage.emit (events.js:166:7) at endReadableNT (_stream_readable.js:905:12) at nextTickCallbackWith2Args (node.js:478:9) at process._tickCallback (node.js:392:17)

msroest commented 8 years ago

Looking at the stack trace closer this actually seems to be an issue with wink-js. I'm working on a branch to handle the exception out of wink-js

msroest commented 8 years ago

I've created a PR in https://github.com/winfinit/wink-js/pull/4 for fixing the underlying issue in wink-js. I need to spend more time learning the node.js async error handling to see how to handle this on the homebridge-wink side if the wink-js issue doesn't get fixed.

pdlove commented 8 years ago

Thank you. Looks like wink-js needs a new npm publish as well. The one on the wink-js github fixes the issue of the huge data dump to the console by putting it into a debug output instead of console.

msroest commented 8 years ago

Yah I asked in the issue we'll see if he gets it out.

pdlove commented 8 years ago

Looks like the npm module has been updated. I think you just need to run "npm update" on homebridge?

msroest commented 8 years ago

Nope an update doesn't work. the packages.json is defining ^0.0.9. The wink-js update was done as version 0.1.0 and so it doesn't update to it. I've open a PR to fix this we'll need a new release though.

msroest commented 8 years ago

Well according to https://www.npmjs.com/package/semver the ^0.1.0 will get be 0.1.0 <= ver < 0.2.0 I guess it depends on how @winfinit is planning on versioning wink-js. Based on the semver info ^0.1.0 is correct as 0.1.x should guarantee no breaking api changes where >0.1.0 things could break as a 0.2.0 could have a incompatible API change.

pdlove commented 8 years ago

That makes a lot more sense. Thanks for that explanation.

winfinit commented 8 years ago

Although I am not fully complying to "semver", however I do strongly agree with "semver" summary:

so you can feel safe to update MINOR version.

msroest commented 8 years ago

Great thanks @winfinit so I'll update it to be 0.1.0 < ver < 1.0.0 then.

KraigM commented 8 years ago

Released in 1.0.3