ptarmiganlabs/butler-cw

Throwing broken promise warning with MQTT disabled

ZachPruckowski opened this issue · 2 comments

ZachPruckowski commented

Describe the bug
I got the following Warning when running Butler-CW with MQTT disabled. I looked into it, and it looks in loadAppIntoCache it doesn't check for whether or not MQTT is enabled when it runs mqttClient.publish() on line 184. I think it needs to have the sort of if statement checking if mqttConfig.out.enable is set that we see on line 331.

(node:15104) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'publish' of undefined
    at [REDACTED]\index.js:184:24
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:15104) 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:15104) [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.

To Reproduce
Disable MQTT and don't put a valid URL in mqttconfig.broker.uri, then try to cache-warm an app.

Expected behavior
Ideally with the MQTT server disabled it wouldn't attempt to publish to it and we wouldn't get this warning.

Describe environment:

  • OS: Windows Server 2019
  • Containerisation - No
  • Version of Butler CW used - 4.0.2
  • Command used to start Butler CW - node index.js

Config file(s)

mqttConfig:
  out:
    enable: false
    tzFormat: UTC
    baseTopic: butler-cw/
  broker:
    uri: mqtt://0.0.0.0
mountaindude commented

Ouch.

I suspect this is something that has surfaced when using Butler CW with recent Node.js versions.
Previously those unhandled exceptions were handled by Node.js, but that's no longer the case I believe (need to check that..).

Anyways, it's obviously a bug that should be fixed. Should be an easy one.. I'll try to find some time during coming couple of weeks.
Or: if you feel up to it I am happily accepting pull requests, of course!

Happy holidays,
/G

mountaindude commented

@ZachPruckowski

This should now be fixed in v4.0.3, which is available on the releases page.