Solve the "no response timeout" issue

CliffS commented 4 years ago

When the bridge does not respond in time, a Promise is rejected with no response timeout. However, that rejection comes outside of the original Promise and thus causes a Unhandled Promise Rejection and the flow of the program is broken.

This patch replaces the setTimeout() with a Bluebird Promise.delay() and thus the eventual rejection is inside the program flow.

Because the delay() tests the state of the Promise, it was necessary to add the delayBetweenCommands value to the timeout to avoid false timeouts.

CliffS commented 4 years ago

This still fails though far less often. I think it's now a race condition between the Promise.delay() and wrapping the resultant promise in promise.reflect().

Zer0x00 commented 4 years ago

Doesn't work either - I'm trying to use homebridge-milight Please see https://github.com/dotsam/homebridge-milight/issues/63

[9/21/2019, 5:06:42 PM] [MiLight] [192.168.0.101:8899] no response timeout
  EventedHTTPServer [::ffff:192.168.0.227] HTTP request: /characteristics +6s
  HAPServer [CC:22:3D:E3:CE:31] HAP Request: PUT /characteristics +1ms
  Accessory [Homebridge-Dev] Processing characteristic set: [{"aid":2,"iid":11,"value":100},{"aid":2,"iid":10,"value":1}] +1ms
  Accessory [Homebridge-Dev] Setting Characteristic "Brightness" to value 100 +1ms
[9/21/2019, 5:06:45 PM] [MiLight] [Wohnzimmer] Setting power state to on
  EventedHTTPServer [::ffff:192.168.0.227] Sending HTTP event '2.10' with data: {"characteristics":[{"aid":2,"iid":10,"value":true}]} +21ms
[9/21/2019, 5:06:45 PM] [MiLight] [Wohnzimmer] Setting brightness to 100
  EventedHTTPServer [::ffff:192.168.0.227] Muting event '2.11' notification for this connection since it originated here. +5ms
  Accessory [Homebridge-Dev] Setting Characteristic "On" to value 1 +0ms
[9/21/2019, 5:06:45 PM] [MiLight] [Wohnzimmer] Omitting 'on' command as we've sent it to this bulb most recently
  EventedHTTPServer [::ffff:192.168.0.227] HTTP Response is finished +3ms
  EventedHTTPServer [::ffff:192.168.0.227] Writing pending HTTP event data +0ms
Unhandled rejection Error: no response timeout
    at /root/node_modules/node-milight-promise/src/milight-v6-mixin.js:132:28
    at tryCatcher (/root/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/root/node_modules/bluebird/js/release/promise.js:517:31)
    at Promise._settlePromise (/root/node_modules/bluebird/js/release/promise.js:574:18)
    at Promise._settlePromise0 (/root/node_modules/bluebird/js/release/promise.js:619:10)
    at Promise._settlePromises (/root/node_modules/bluebird/js/release/promise.js:699:18)
    at Promise._fulfill (/root/node_modules/bluebird/js/release/promise.js:643:18)
    at Timeout._onTimeout (/root/node_modules/bluebird/js/release/timers.js:26:46)
    at ontimeout (timers.js:436:11)
    at tryOnTimeout (timers.js:300:5)
    at listOnTimeout (timers.js:263:5)
    at Timer.processTimers (timers.js:223:10)

Unhandled rejection Error: no response timeout
    at /root/node_modules/node-milight-promise/src/milight-v6-mixin.js:132:28
    at tryCatcher (/root/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/root/node_modules/bluebird/js/release/promise.js:517:31)
    at Promise._settlePromise (/root/node_modules/bluebird/js/release/promise.js:574:18)
    at Promise._settlePromise0 (/root/node_modules/bluebird/js/release/promise.js:619:10)
    at Promise._settlePromises (/root/node_modules/bluebird/js/release/promise.js:699:18)
    at Promise._fulfill (/root/node_modules/bluebird/js/release/promise.js:643:18)
    at Timeout._onTimeout (/root/node_modules/bluebird/js/release/timers.js:26:46)
    at ontimeout (timers.js:436:11)
    at tryOnTimeout (timers.js:300:5)
    at listOnTimeout (timers.js:263:5)
    at Timer.processTimers (timers.js:223:10)

CliffS commented 4 years ago

I have changed this pull request to resolve() rather than reject() on no response timeout.

The reason is:

There is a race condition that there is a race condition meaning that this can reject before the Promise.reflect() is applied.
After the Promise.reflect() the result of the promise is effectively thrown away so it doesn't matter whether it resolves or rejects.

I think this finally closes this issue.

Zer0x00 commented 4 years ago

I have changed this pull request to resolve() rather than reject() on no response timeout.

The reason is:

There is a race condition that there is a race condition meaning that this can reject before the Promise.reflect() is applied.

After the Promise.reflect() the result of the promise is effectively thrown away so it doesn't matter whether it resolves or rejects.

I think this finally closes this issue.

This fixed the error. Thank you! But somehow the plugin is nevertheless not working

CliffS commented 4 years ago

But somehow the plugin is nevertheless not working

What sort of not working?

Zer0x00 commented 4 years ago

But somehow the plugin is nevertheless not working

What sort of not working?

Sorry for confusion, talking about homebridge-milight.

I discovered an other plugin (homebridge-milight-v6) which works, trying to port it back to homebridge-milight.

homebridge-milight sends following command: [ 49, 0, 0, 10, 6, 2, 0, 0, 0, 2 ]

homebridge-milight-v6 sends [ 49, 0, 0, 10, 6, 1, 0, 0, 0, 1 ]

Could you tell me how to change it?

mwittig / node-milight-promise

Solve the "no response timeout" issue #67