homebridge / ciao

RFC 6762 and RFC 6763 compliant mdns service discovery library written in Typescript
MIT License
79 stars 6 forks source link

homebridge-beta: unhandled exception thrown by Ciao #2

Closed adriancable closed 4 years ago

adriancable commented 4 years ago

Testing homebridge-beta. Ciao throws an unhandled exception if you run avahi-browse -at on the same device that is running homebridge.

[2296]: [6/13/2020, 11:00:47 AM] Error: Truncated messages are currently unsupported [2296]: at Responder.handleQuery (/root/node_modules/homebridge/node_modules/@homebridge/ciao/lib/Responder.js:232:19) [2296]: at MDNSServer.handleMessage (/root/node_modules/homebridge/node_modules/@homebridge/ciao/lib/MDNSServer.js:243:26) [2296]: at Socket.emit (events.js:315:20) [2296]: at UDP.onMessage (dgram.js:908:8) [2296]: [6/13/2020, 11:00:47 AM] Got SIGTERM, shutting down Homebridge...

-Adrian

oznu commented 4 years ago

@Supereg

Supereg commented 4 years ago

Iā€˜m aware of this, working to implement support for this. Thanks for reporting

Supereg commented 4 years ago

Just never experienced this on my end, thus wasn't my highest priority at the time.

adriancable commented 4 years ago

Hi Andi - just to clarify, I know that Ciao is still under development and wasn't asking for support for specific things. But, it should not throw unhandled exceptions and crash Homebridge, so I think before it's supported just add a try/catch block to handle this and other unanticipated things.

-Adrian

On Jun 14, 2020, at 06:12, Andi notifications@github.com wrote:

ļ»æ Just never experienced this on my end, thus wasn't my highest priority at the time.

ā€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Supereg commented 4 years ago

You are right, of course. This is something which slipped through from the early devs days. Going to push an update soon, which will either just ignore such packets and print a warning or is adding support for truncated queries.

adriancable commented 4 years ago

@Supereg - that is awesome. Thank you!

Supereg commented 4 years ago

Took me a little more time than expected to get back working at this. Truncated queries, are now supported as of https://github.com/homebridge/ciao/commit/0d90d628c13f9e4f9249efdb16ca2ea4363af6d2. Will be part of the next beta dropping in the next few hours. Still want to ship some other improvements.

adriancable commented 4 years ago

@Supereg - that is awesome. I am very excited to see what you're doing with Homebridge right now - you've given it a new lease of life.

Supereg commented 4 years ago

alright, schedule didn't go as planned, got caught up rewriting the dns packet encoder/decoder šŸ˜… Released a new beta 1.0.0-beta.17 which includes support for truncated queries. Anything else planned will be postponed to the next beta release.

Supereg commented 4 years ago

Forgot to mention, not releasing a new homebridge/hap-nodejs for this as reinstalling homebridge/hap-nodejs beta will just pull in the latest ciao version. So just reinstall homebrideg@beta and ciao will be updated to beta 17.

adriancable commented 4 years ago

Just tested the new Ciao and it works great with truncated queries. Great job! Apart from the usual regression testing, are there any reasons you can think of to still use bonjour-hap?

-Adrian

From: Andi notifications@github.com Reply to: homebridge/ciao reply@reply.github.com Date: Friday, June 19, 2020 at 7:48 AM To: homebridge/ciao ciao@noreply.github.com Cc: Adrian Cable adrian.cable@gmail.com, Author author@noreply.github.com Subject: Re: [homebridge/ciao] homebridge-beta: unhandled exception thrown by Ciao (#2)

Forgot to mention, not releasing a new homebridge/hap-nodejs for this as reinstalling homebridge/hap-nodejs beta will just pull in the latest ciao version. So just reinstall homebrideg@beta and ciao will be updated to beta 17.

ā€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Supereg commented 4 years ago

Apart from the usual regression testing, are there any reasons you can think of to still use bonjour-hap?

absolutely not. burry that thing šŸ˜… well, ciao does not have a browser currently, so if you need that functionality, maybe. but the bonjour-hap browser is also broken (or rather not properly implemented) in so many ways.