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.

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

Crash upon removing bridge from Home #22

Closed
nikolaykasyanov opened this issue Jul 31, 2018 · 14 comments · Fixed by homebridge/HAP-NodeJS#655
Closed

Crash upon removing bridge from Home #22

nikolaykasyanov opened this issue Jul 31, 2018 · 14 comments · Fixed by homebridge/HAP-NodeJS#655
Labels
bug 🐛 There is at least high chance that it is a bug!

Comments

@nikolaykasyanov
Copy link

Hi, thanks for the fork!

I'm experiencing a crash upon removing bridges from Home, here's the stacktrace:

31 Jul 19:40:15 - TypeError: Cannot read property 'updateAdvertisement' of undefined
    at Bridge.Accessory._handleUnpair (/Users/nkasyanov/Desktop/node-red/node_modules/hap-nodejs/lib/Accessory.js:699:20)
    at HAPServer.emit (events.js:182:13)
    at HAPServer._handlePairings (/Users/nkasyanov/Desktop/node-red/node_modules/hap-nodejs/lib/HAPServer.js:823:10)
    at HAPServer.<anonymous> (/Users/nkasyanov/Desktop/node-red/node_modules/hap-nodejs/lib/HAPServer.js:209:39)
    at IncomingMessage.emit (events.js:182:13)
    at endReadableNT (_stream_readable.js:1081:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
@nikolaykasyanov
Copy link
Author

Here are some observations:

  • it's not 100% reproducible
  • happens inside Docker and without Docker

@ttww
Copy link

ttww commented Dec 27, 2018

Just for your information: Got the same problem after removing a bridge.
Restarting node-red "used" as a work around.

@martincollar
Copy link

same here, when i remove bridge from home node-red crashes

20 Jan 19:44:06 - TypeError: Cannot read property 'updateAdvertisement' of undefined at Bridge.Accessory._handleUnpair (/Users/kolar/.node-red/node_modules/@plasma2450/hap-nodejs/lib/Accessory.js:699:19) at emitTwo (events.js:125:13) at HAPServer.emit (events.js:213:7) at HAPServer._handlePairings (/Users/kolar/.node-red/node_modules/@plasma2450/hap-nodejs/lib/HAPServer.js:823:10) at HAPServer.<anonymous> (/Users/kolar/.node-red/node_modules/@plasma2450/hap-nodejs/lib/HAPServer.js:209:39) at emitNone (events.js:105:13) at IncomingMessage.emit (events.js:207:7) at endReadableNT (_stream_readable.js:1045:12) at _combinedTickCallback (internal/process/next_tick.js:138:11) at process._tickCallback (internal/process/next_tick.js:180:9)

@1stone
Copy link

1stone commented Feb 3, 2019

I'm experincing the same problem:

Feb 03 11:14:00 openhabian Node-RED[2227]: 3 Feb 11:14:00 - TypeError: Cannot read property 'updateAdvertisement' of undefined
Feb 03 11:14:00 openhabian Node-RED[2227]:     at Bridge.Accessory._handleUnpair (/home/nodered/.node-red/node_modules/hap-nodejs/lib/Accessory.js:701:20)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at emitTwo (events.js:126:13)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at HAPServer.emit (events.js:214:7)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at HAPServer._handlePairings (/home/nodered/.node-red/node_modules/hap-nodejs/lib/HAPServer.js:823:10)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at HAPServer.<anonymous> (/home/nodered/.node-red/node_modules/hap-nodejs/lib/HAPServer.js:209:39)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at emitNone (events.js:106:13)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at IncomingMessage.emit (events.js:208:7)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at endReadableNT (_stream_readable.js:1064:12)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at _combinedTickCallback (internal/process/next_tick.js:138:11)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at process._tickCallback (internal/process/next_tick.js:180:9)

Somehow this creates some inconsistent state, as I'm unable to add that bridge (after modifications) again to the Home App. The only thing that worked so far, is deleteing the Bridge-Node in NR and creating it new again. Unfortunately, that involves adjusting all Service-Nodes...

If someone knows about a better workaround, please let me know.

@martincollar
Copy link

im using homekit2mqtt and integrating it with node-red via mqtt, this node red plugin looks broken :/

@Shaquu
Copy link
Member

Shaquu commented Feb 23, 2019 •
edited
Loading

@martincollar in your stack trace I can see you are using fork by @plasma2450.
Please remove @plasma2450/node-red-contrib-homekit-bridged and @plasma2450/hap-nodes.

Then download our latest version (released today):
npm i node-red-contrib-homekit-bridged

@1stone Could you update to 0.5.0 and try to reproduce error?
Can you write complete list of steps to reproduce it?

@Shaquu
Copy link
Member

Shaquu commented Feb 27, 2019

I can say the problem does exists ;)
I am going to look at this next week.

@Shaquu Shaquu added this to the Release 0.7.0 milestone Mar 8, 2019
@Shaquu Shaquu mentioned this issue Mar 18, 2019
Closed
@Shaquu
Copy link
Member

Shaquu commented Mar 18, 2019

There will be a try to fix this today. Could be related to #12

@Shaquu
Copy link
Member

Shaquu commented Mar 18, 2019

@nikolaykasyanov @ttww @martincollar @1stone
I think I fixed it on my last commit to dev branch.

What was the problem?
When we remove Service nodes from flow then we usually still have unused Bridge node.
Bridge node is not published since it doesn't have active Service nodes. And since it's not published it crash on advertiser update because advertiser is not started!

What I did?
I overrided unpair event handler for Bridge server and added check for advertiser if it's null.
It works for me! :)

Shaquu added a commit that referenced this issue Mar 18, 2019
@Shaquu
Possible fix for #22 travis build
5e70dfa
@Shaquu Shaquu mentioned this issue Mar 18, 2019
Merged
@Shaquu
Copy link
Member

Shaquu commented Mar 18, 2019

I made a PR in HAP-nodejs. If they accept it then I will remove override from our code.

@Shaquu
Copy link
Member

Shaquu commented Mar 19, 2019 •
edited
Loading

Retest:

  1. Replicating Issue
  • Use version from master branch
  • Create new Parent Service with Bridge
  • Deploy in node-red
  • Add Bridge to Home.app
  • Remove Parent Service from flow
  • Deploy in node-red
  • Remove Bridge from Home.app
  • node-red should crash with error 'TypeError: Cannot read property 'updateAdvertisement' of undefined'
  1. Checking fix
  • Use version from dev branch (you can run module in debug mode DEBUG=NRCHKB node-red)
  • Create new Parent Service with Bridge
  • Deploy in node-red
  • Add Bridge to Home.app
  • Remove Parent Service from flow
  • Deploy in node-red
  • Remove Bridge from Home.app
  • node-red should unpair successfully (in debug there should be information about this)

Please retest using above.
I summon my best associates @radokristof @crxporter @flic @radionoise @emilingerslev aka @NRCHKB/test

@Shaquu Shaquu added the bug 🐛 There is at least high chance that it is a bug! label Mar 19, 2019
@Shaquu
Copy link
Member

Shaquu commented Mar 20, 2019

The last thing I want to do is to release something tested by me only. But if I have to...

@Shaquu Shaquu removed this from the Release 0.7.0 milestone Mar 20, 2019
@Shaquu Shaquu mentioned this issue Mar 20, 2019
Merged
@Shaquu Shaquu closed this as completed in 8168bc6 Mar 20, 2019
Shaquu added a commit that referenced this issue Mar 20, 2019
@Shaquu
Merge pull request #71 from NRCHKB/dev
4e31fa4 
Release 0.6.1

- Fixes for #22 and #66 
- Some renaming to our new organisation name
- reintroduced debug mode for our module, run with DEBUG=NRCHKB node-red
- some small changes to process of finding Parent Service as Linked Service
@Shaquu
Copy link
Member

Shaquu commented Mar 20, 2019

Please update to new version 0.6.1 using node-red.
Then please retest this bug if it's still occurs.

@Shaquu Shaquu reopened this Mar 20, 2019
@Shaquu Shaquu closed this as completed Mar 24, 2019
@Shaquu
Copy link
Member

Shaquu commented Apr 6, 2019

So as my fix is merged to newest HAP-nodejs then we need to remove fix from our code.
Opening so I will not forget.

@Shaquu Shaquu reopened this Apr 6, 2019
Shaquu added a commit to Shaquu/node-red-contrib-homekit-bridged that referenced this issue Apr 25, 2019
@Shaquu
Remove modified handleUnpair due to reasons in NRCHKB#22
b1e91ce
@Shaquu Shaquu mentioned this issue Apr 25, 2019
Merged
Shaquu added a commit that referenced this issue Apr 25, 2019
@Shaquu
Merge pull request #105 from Shaquu/dev
82db873 
Remove modified handleUnpair due to reasons in #22
@Shaquu Shaquu closed this as completed Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐛 There is at least high chance that it is a bug!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

During unpair make sure _advertiser is not null
5 participants
@nikolaykasyanov @Shaquu @martincollar @ttww @1stone