From 7c435f51ce42aa04dc0fcdb30f1e2d3f2e49e319 Mon Sep 17 00:00:00 2001 From: Adrian Cable Date: Thu, 21 Apr 2022 07:35:39 -0700 Subject: [PATCH] Fix race condition causing rare connection lock up. --- README.md | 2 +- lib/nest-connection.js | 219 +++++++++++++++++------------------------ package.json | 4 +- 3 files changed, 95 insertions(+), 130 deletions(-) diff --git a/README.md b/README.md index b664e58..089e51e 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![verified-by-homebridge](https://badgen.net/badge/homebridge/verified/purple)](https://github.com/homebridge/homebridge/wiki/Verified-Plugins) [![Discord](https://img.shields.io/discord/432663330281226270?color=728ED5&logo=discord&label=discord)](https://discord.gg/j5WwJTB) -Nest plug-in for [Homebridge](https://github.com/nfarina/homebridge) using the native Nest API. See what's new in [release 4.6.5](https://github.com/chrisjshull/homebridge-nest/releases/tag/v4.6.5). +Nest plug-in for [Homebridge](https://github.com/nfarina/homebridge) using the native Nest API. See what's new in [release 4.6.6](https://github.com/chrisjshull/homebridge-nest/releases/tag/v4.6.6). Integrate your Nest Thermostat, Temperature Sensors, Nest Protect, and Nest x Yale Lock devices into your HomeKit system. Both Nest Accounts (pre-August 2019) and Google Accounts are supported. diff --git a/lib/nest-connection.js b/lib/nest-connection.js index abbbd3f..8745bd8 100644 --- a/lib/nest-connection.js +++ b/lib/nest-connection.js @@ -31,9 +31,6 @@ const API_AUTH_FAIL_RETRY_DELAY_SECONDS = 15; // Delay after authentication fail (long) before retrying const API_AUTH_FAIL_RETRY_LONG_DELAY_SECONDS = 60 * 60; -// Interval between Nest subscribe requests -const API_SUBSCRIBE_DELAY_SECONDS = 0.1; - // Nest property updates are combined together if less than this time apart, to reduce network traffic const API_PUSH_DEBOUNCE_SECONDS = 2; @@ -98,11 +95,7 @@ class Connection { this.legacyDeviceMap = {}; this.protobufBody = {}; this.lastProtobufCode = null; - this.pollTimes = {}; - this.transcoderProcesses = []; - this.associatedStreamers = []; this.preemptiveReauthTimer = null; - this.cancelObserve = null; this.connectionFailures = 0; this.pendingUpdates = []; this.mergeUpdates = []; @@ -436,14 +429,6 @@ class Connection { }); }, (isGoogle ? API_GOOGLE_REAUTH_MINUTES : API_NEST_REAUTH_MINUTES) * 60 * 1000); - this.associatedStreamers.forEach(streamer => { - try { - streamer.onTheFlyReauthorize(); - } catch (error) { - this.verbose('Warning: attempting to reauthorize with expired streamer', streamer); - } - }); - return true; } } @@ -515,7 +500,20 @@ class Connection { return new Promise((res, rej) => { let promiseCompleted = false; + const attempt = (func, errorText) => { + try { + func(); + } catch(error) { + this.debug(errorText, error); + } + }; + const cleanAndResolve = val => { + if (promiseCompleted) { + return; + } + promiseCompleted = true; + clearTimeout(this.timeoutTimer); clearInterval(this.cancelObserveTimer); clearInterval(this.pingTimer); @@ -523,25 +521,20 @@ class Connection { this.cancelObserveTimer = null; this.pingTimer = null; - try { - req.destroy(); - } catch(error) { - // Ignore - } - - try { - client.destroy(); - } catch(error) { - // Ignore - } + attempt(() => req.removeAllListeners(), 'cleanAndReject: request remove listeners error -'); + attempt(() => client.removeAllListeners(), 'cleanAndReject: client remove listeners error -'); + attempt(() => req.destroy(), 'cleanAndReject: request destroy error -'); + attempt(() => client.destroy(), 'cleanAndReject: client destroy error -'); - if (!promiseCompleted) { - promiseCompleted = true; - res(val); - } + res(val); }; const cleanAndReject = val => { + if (promiseCompleted) { + return; + } + promiseCompleted = true; + clearTimeout(this.timeoutTimer); clearInterval(this.cancelObserveTimer); clearInterval(this.pingTimer); @@ -549,22 +542,12 @@ class Connection { this.cancelObserveTimer = null; this.pingTimer = null; - try { - req.destroy(); - } catch(error) { - // Ignore - } + attempt(() => req.removeAllListeners(), 'cleanAndResolve: request remove listeners error -'); + attempt(() => client.removeAllListeners(), 'cleanAndResolve: client remove listeners error -'); + attempt(() => req.destroy(), 'cleanAndResolve: request destroy error -'); + attempt(() => client.destroy(), 'cleanAndResolve: client destroy error -'); - try { - client.destroy(); - } catch(error) { - // Ignore - } - - if (!promiseCompleted) { - promiseCompleted = true; - rej(val); - } + rej(val); }; const timeoutFunction = () => { @@ -655,18 +638,6 @@ class Connection { } }, 1000); - // this.cancelObserve = () => req.close(http2.constants.NGHTTP2_CANCEL); - this.cancelObserve = () => { - req.close(http2.constants.NGHTTP2_CANCEL); - client.destroy(); - }; - - /* req.on('response', (headers, flags) => { - for (const name in headers) { - console.log(`${name}: ${headers[name]}`); - } - }); */ - req.on('data', data => { if (this.timeoutTimer) { clearTimeout(this.timeoutTimer); @@ -1252,85 +1223,79 @@ class Connection { return data; } - dataTimerLoop(resolve, handler) { + async dataTimerLoop(resolve, handler) { var notify = resolve || handler; - var apiLoopTimer; - this.updateData().then(data => { - if (data) { - this.verbose('API subscribe GET: got updated data'); - notify(data); - } - }).catch(error => { - error.status = error.response && error.response.status; - if (!['ESOCKETTIMEDOUT','ECONNABORTED'].includes(error.code)) { - if (!error.status || error.status != 401) { - // 401 responses are normal when reauthentication is required - not actually a real "error" - this.error('Nest API call to subscribe to device settings updates returned an error: ' + (error.status || error.code || error)); + // eslint-disable-next-line + while (true) { + try { + let data = await this.updateData(); + if (data) { + this.verbose('API subscribe GET: got updated data'); + notify(data); } - if (error.status == 401 || error.status == 403 || ['ECONNREFUSED','ENETUNREACH'].includes(error.code)) { - // Token has probably expired, or transport endpoint has changed - re-authenticate - this.log('Reauthenticating on Nest service ...'); - return this.auth().catch(() => { - this.log('Reauthentication failed, waiting for ' + API_AUTH_FAIL_RETRY_DELAY_SECONDS + ' seconds.'); - return Promise.delay(API_AUTH_FAIL_RETRY_DELAY_SECONDS * 1000); - }); - } else { - this.log('Retrying in ' + API_RETRY_DELAY_SECONDS + ' seconds.'); - return Promise.delay(API_RETRY_DELAY_SECONDS * 1000); + } catch (error) { + error.status = error.response && error.response.status; + if (!['ESOCKETTIMEDOUT', 'ECONNABORTED'].includes(error.code)) { + if (!error.status || error.status != 401) { + // 401 responses are normal when reauthentication is required - not actually a real "error" + this.error('Nest API call to subscribe to device settings updates returned an error: ' + (error.status || error.code || error)); + } + if (error.status == 401 || error.status == 403 || ['ECONNREFUSED', 'ENETUNREACH'].includes(error.code)) { + // Token has probably expired, or transport endpoint has changed - re-authenticate + this.log('Reauthenticating on Nest service ...'); + try { + await this.auth(); + } catch (error) { + this.log('Reauthentication failed, waiting for ' + API_AUTH_FAIL_RETRY_DELAY_SECONDS + ' seconds.'); + await Promise.delay(API_AUTH_FAIL_RETRY_DELAY_SECONDS * 1000); + } + } else { + this.log('Retrying in ' + API_RETRY_DELAY_SECONDS + ' seconds.'); + await Promise.delay(API_RETRY_DELAY_SECONDS * 1000); + } } } - }).finally(() => { - apiLoopTimer = setInterval(() => { - if (apiLoopTimer) { - clearInterval(apiLoopTimer); - } - this.dataTimerLoop(null, handler); - }, API_SUBSCRIBE_DELAY_SECONDS * 1000); - }); + } } - protobufDataTimerLoop(resolve, handler) { - var apiLoopTimer; - - this.verbose('API observe POST: streaming request initiated'); - this.updateProtobufData(resolve, handler).then(() => { - this.verbose('API observe POST: streaming request concluded'); - // Token has probably expired, or transport endpoint has changed - re-authenticate - // console.log(this.lastProtobufCode); - // code 4: context timed out - // code 7: invalid authentication - // code 8: message quota exceeded - // code 13: internal error encountered - // code 14: socket closed / OS error - if (this.lastProtobufCode && this.lastProtobufCode.code == 13) { - this.error('API observe: internal error, waiting for ' + API_RETRY_DELAY_SECONDS + ' seconds, code', this.lastProtobufCode); + async protobufDataTimerLoop(resolve, handler) { + // eslint-disable-next-line + while (true) { + this.verbose('API observe POST: streaming request initiated'); + try { + await this.updateProtobufData(resolve, handler); + this.verbose('API observe POST: streaming request concluded'); + // Token has probably expired, or transport endpoint has changed - re-authenticate + // console.log(this.lastProtobufCode); + // code 4: context timed out + // code 7: invalid authentication + // code 8: message quota exceeded + // code 13: internal error encountered + // code 14: socket closed / OS error + if (this.lastProtobufCode && this.lastProtobufCode.code == 13) { + this.error('API observe: internal error, waiting for ' + API_RETRY_DELAY_SECONDS + ' seconds, code', this.lastProtobufCode); + await Promise.delay(API_RETRY_DELAY_SECONDS * 1000); + } else if (this.lastProtobufCode && this.lastProtobufCode.code == 7) { // Was != 4 + this.log('Reauthenticating on Nest service ...'); + try { + await this.auth(); + } catch(error) { + this.log('Reauthentication failed, waiting for ' + API_AUTH_FAIL_RETRY_DELAY_SECONDS + ' seconds'); + await Promise.delay(API_AUTH_FAIL_RETRY_DELAY_SECONDS * 1000); + } + } else { + this.verbose('API observe: resolving null, code', this.lastProtobufCode); + this.lastProtobufCode = null; + resolve(null); + } + } catch(error) { + this.error('API observe: error', error); + this.error('^^^^^ this message is for information only, it does not mean there is a problem, please do not file a ticket unless you actually have a problem with the function of the plug-in'); + this.error('Retrying in ' + API_RETRY_DELAY_SECONDS + ' seconds.'); return Promise.delay(API_RETRY_DELAY_SECONDS * 1000); - } else if (this.lastProtobufCode && this.lastProtobufCode.code == 7) { // Was != 4 - this.log('Reauthenticating on Nest service ...'); - return this.auth().catch(() => { - this.log('Reauthentication failed, waiting for ' + API_AUTH_FAIL_RETRY_DELAY_SECONDS + ' seconds.'); - return Promise.delay(API_AUTH_FAIL_RETRY_DELAY_SECONDS * 1000); - }); - } else { - this.verbose('API observe: resolving null, code', this.lastProtobufCode); - this.lastProtobufCode = null; - return Promise.resolve(null); } - }).catch(error => { - this.error('API observe: error', error); - this.error('^^^^^ this message is for information only, it does not mean there is a problem, please do not file a ticket unless you actually have a problem with the function of the plug-in'); - this.error('Retrying in ' + API_RETRY_DELAY_SECONDS + ' seconds.'); - return Promise.delay(API_RETRY_DELAY_SECONDS * 1000); - }).finally(() => { - this.verbose('API observe: setting issue timer.'); - apiLoopTimer = setInterval(() => { - if (apiLoopTimer) { - clearInterval(apiLoopTimer); - } - this.protobufDataTimerLoop(null, handler); - }, API_SUBSCRIBE_DELAY_SECONDS * 1000); - }); + } } subscribe(handler) { diff --git a/package.json b/package.json index 2749181..a65c8c0 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "prepublishOnly": "npm run lint", "preversion": "npm run lint" }, - "version": "4.6.5", + "version": "4.6.6", "warnings": [ { "code": "ENOTSUP", @@ -45,7 +45,7 @@ "node": ">=7.0.0", "homebridge": ">=0.2.5" }, - "pkgid": "homebridge-nest@4.6.5" + "pkgid": "homebridge-nest@4.6.6" } ] }