HAP-NodeJS icon indicating copy to clipboard operation
HAP-NodeJS copied to clipboard

Avahi/DBus: Shutdown of avahi stays undetected and leads to UnhandledPromiseRejection on shutdown.

Open bauer-andreas opened this issue 3 years ago • 9 comments

Analysis

The current avahi advertiser implementation cannot detect if the avahi-deamon is rebooted or stopped. The service advertisement will be removed and will stay removed even if avahi starts again. Additionally, somewhere in the code is a UnhandledPromiseRejection triggered when HAP-NodeJS shuts down. Presumably this has to do with the advertiser trying to clean up its records, but failing to do so(?).

I haven't tested what happens if a txt record update is triggered while avahi is stoped or when avahi as restarted.

Expected Behavior

Ideally, we could detect a shutdown of avahi and also be notified once it is up and running again, such that we can readvertise our service records. I think, as soon as we detect that there is a running avahi instance, it is fine to assume it will be started at some point (e.g. might just be a restart).

Steps To Reproduce

  1. Open mDNS monitoring app like Discovery
  2. Start HAP-NodeJS with AVAHI advertiser
  3. Observe service advertisement
  4. Stop/restart avahi
  5. See service advertisement being removed
  6. ...
  7. Stop HAP-NodeJS and observe the promise rejection warning below.

Logs

Received MESSAGE: {"serial":6,"destination":":1.190","errorName":"org.freedesktop.DBus.Error.UnknownObject","replySerial":6,"signature":"s","sender":":1.191","type":3,"flags":1,"body":["Method \"Free\" with signature \"\" on interface \"org.freedesktop.Avahi.EntryGroup\" doesn't exist\n"]}

(node:9242) UnhandledPromiseRejectionWarning: [object Array]
    at emitUnhandledRejectionWarning (internal/process/promises.js:170:15)
    at processPromiseRejections (internal/process/promises.js:247:11)
    at processTicksAndRejections (internal/process/task_queues.js:96:32)
(node:9242) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:9242) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
    at emitDeprecationWarning (internal/process/promises.js:180:11)
    at processPromiseRejections (internal/process/promises.js:249:13)
    at processTicksAndRejections (internal/process/task_queues.js:96:32)

Configuration

-

Environment

  • OS: Linux 5.10.63-v7+
  • Software: 0.10.0-beta.7
  • Node: v14.18.2
  • npm: 6.14.15

Process Supervisor

not applicable

Additional Context

The Received MESSAGE ... log output is custom made, and prints anything arriving on the bus.connection.on("message") event.

bauer-andreas avatar Jan 06 '22 19:01 bauer-andreas

CC: @adriancable, @oznu

bauer-andreas avatar Jan 06 '22 19:01 bauer-andreas

Hi @Supereg - I would like to re-open this and provide a suggested solution, please. I actually think this is quite important. If avahi crashes for any reason, then it will be restarted by systemd but HAP-NodeJS will not know, and so cannot re-advertise - so from the user's perspective the service will just disappear.

The solution is something like this in the constructor for AvahiAdvertiser:

this.bus.getService('org.freedesktop.Avahi').getInterface('/', 'org.freedesktop.Avahi.Server', (err, iface) => {
  if (iface) {
    iface.on('StateChanged', state => {
      if (this.path && state == 2) {
        // Already advertising, so need to re-advertise on Avahi restart
        this.startAdvertising();
      }
    });
  }
});

The states are:

0: collision 1: registering 2: running 3: connecting 4: failure

When Avahi starts up, it transitions to state 1 then state 2. So catching StateChanged to 2 will happen whenever Avahi is (re)started and is the correct signal to (re)advertise.

adriancable avatar Jan 19 '22 21:01 adriancable

@Supereg - thanks. Are you OK to add this code snippet to a beta and try it out? (I'm on the road for the next few days, so won't be able to make a PR ...)

adriancable avatar Jan 19 '22 21:01 adriancable

@Supereg - thanks. Are you OK to add this code snippet to a beta and try it out? (I'm on the road for the next few days, so won't be able to make a PR ...)

0.10.0 beta is frozen as we prepare for the release. Don't really want to push last minute changes. But will include it in the oncoming beta cycle.

bauer-andreas avatar Jan 20 '22 12:01 bauer-andreas

@Supereg - perfect!

adriancable avatar Jan 20 '22 15:01 adriancable

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Feb 21 '22 11:02 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Mar 24 '22 11:03 github-actions[bot]

@adriancable

Where do you got the state id mapping from, looking at https://github.com/lathiat/avahi/blob/fd482a74625b8db8547b8cfca3ee3d3c6c721423/avahi-common/defs.h#L220-L227 I would have driven a different mapping.

Namely: 0: invalid 1: registering 2: running 3: collision 4: failure

See also https://github.com/lathiat/avahi/blob/fd482a74625b8db8547b8cfca3ee3d3c6c721423/avahi-daemon/dbus-protocol.c#L1211-L1238 where the StateChanged event is emitted.

bauer-andreas avatar Sep 17 '22 09:09 bauer-andreas

Addressing issue with #970, review and verification pending.

bauer-andreas avatar Sep 17 '22 10:09 bauer-andreas

This issue is now resolved in HAP-NodeJS version 0.11.0-beta.9.

bauer-andreas avatar Oct 31 '22 06:10 bauer-andreas