From 9794c7f38ec3f57e51d5fbecbd700ba2714a9f41 Mon Sep 17 00:00:00 2001 From: GochoMugo Date: Wed, 25 Jan 2017 17:26:57 +0300 Subject: [PATCH 1/4] src/polling: Fix the Offset Infinite Loop bug References: * Issue #36: https://github.com/yagop/node-telegram-bot-api/issues/36 * answer/solution: https://github.com/yagop/node-telegram-bot-api/issues/36#issuecomment-268532067 --- doc/api.md | 1 + src/telegram.js | 5 +++++ src/telegramPolling.js | 35 ++++++++++++++++++++++++++++++----- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/doc/api.md b/doc/api.md index adb8c25c..f5ae4d22 100644 --- a/doc/api.md +++ b/doc/api.md @@ -122,6 +122,7 @@ Emits `message` when a message arrives. | [options.request] | Object | | Options which will be added for all requests to telegram api. See https://github.com/request/request#requestoptions-callback for more information. | | [options.baseApiUrl] | String | https://api.telegram.org | API Base URl; useful for proxying and testing | | [options.filepath] | Boolean | true | Allow passing file-paths as arguments when sending files, such as photos using `TelegramBot#sendPhoto()`. See [usage information][usage-sending-files-performance] for more information on this option and its consequences. | +| [options.badRejection] | Boolean | false | Set to `true` **if and only if** the Node.js version you're using terminates the process on unhandled rejections. This option is only for *forward-compatibility purposes*. | diff --git a/src/telegram.js b/src/telegram.js index b69fa59e..df94cd86 100644 --- a/src/telegram.js +++ b/src/telegram.js @@ -174,6 +174,10 @@ class TelegramBot extends EventEmitter { * @param {Boolean} [options.filepath=true] Allow passing file-paths as arguments when sending files, * such as photos using `TelegramBot#sendPhoto()`. See [usage information][usage-sending-files-performance] * for more information on this option and its consequences. + * @param {Boolean} [options.badRejection=false] Set to `true` + * **if and only if** the Node.js version you're using terminates the + * process on unhandled rejections. This option is only for + * *forward-compatibility purposes*. * @see https://core.telegram.org/bots/api */ constructor(token, options = {}) { @@ -184,6 +188,7 @@ class TelegramBot extends EventEmitter { this.options.webHook = (typeof options.webHook === 'undefined') ? false : options.webHook; this.options.baseApiUrl = options.baseApiUrl || 'https://api.telegram.org'; this.options.filepath = (typeof options.filepath === 'undefined') ? true : options.filepath; + this.options.badRejection = (typeof options.badRejection === 'undefined') ? false : options.badRejection; this._textRegexpCallbacks = []; this._replyListenerId = 0; this._replyListeners = []; diff --git a/src/telegramPolling.js b/src/telegramPolling.js index d3f102fb..b4fe884e 100644 --- a/src/telegramPolling.js +++ b/src/telegramPolling.js @@ -79,6 +79,18 @@ class TelegramBotPolling { return !!this._lastRequest; } + /** + * Handle error thrown during polling. + * @private + * @param {Error} error + */ + _error(error) { + if (!this.bot.listeners('polling_error').length) { + return console.error('error: [polling_error] %j', err); // eslint-disable-line no-console + } + return this.bot.emit('polling_error', error); + } + /** * Invokes polling (with recursion!) * @return {Promise} promise of the current request @@ -99,12 +111,25 @@ class TelegramBotPolling { }) .catch(err => { debug('polling error: %s', err.message); - if (this.bot.listeners('polling_error').length) { - this.bot.emit('polling_error', err); - } else { - console.error('error: [polling_error] %j', err); // eslint-disable-line no-console + /** + * We need to mark the already-processed items + * to avoid fetching them again once the application + * is restarted, or moves to next polling interval + * (in cases where unhandled rejections do not terminate + * the process). + * See https://github.com/yagop/node-telegram-bot-api/issues/36#issuecomment-268532067 + */ + if (!this.bot.options.badRejection) { + return this._error(err); } - return null; + const opts = { + offset: this.options.params.offset, + limit: 1, + timeout: 0, + }; + return this.bot.getUpdates(opts).then(() => { + return this._error(err); + }); }) .finally(() => { if (this._abort) { From 80bf91b50e1fb8115ecb07f9a34124a7b9a2eb57 Mon Sep 17 00:00:00 2001 From: GochoMugo Date: Wed, 1 Feb 2017 17:54:41 +0300 Subject: [PATCH 2/4] src/polling: Log failure to handle Offset Infinite Loop bug --- src/telegramPolling.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/telegramPolling.js b/src/telegramPolling.js index b4fe884e..8df5d1bb 100644 --- a/src/telegramPolling.js +++ b/src/telegramPolling.js @@ -111,7 +111,7 @@ class TelegramBotPolling { }) .catch(err => { debug('polling error: %s', err.message); - /** + /* * We need to mark the already-processed items * to avoid fetching them again once the application * is restarted, or moves to next polling interval @@ -129,6 +129,21 @@ class TelegramBotPolling { }; return this.bot.getUpdates(opts).then(() => { return this._error(err); + }).catch(requestErr => { + /* + * We have been unable to handle this error. + * We have to log this to stderr to ensure devops + * understands that they may receive already-processed items + * on app restart. + */ + /* eslint-disable no-console */ + const bugUrl = 'https://github.com/yagop/node-telegram-bot-api/issues/36#issuecomment-268532067'; + console.error('error: Internal handling of The Offset Infinite Loop failed'); + console.error(`error: This was due to error '${requestErr}'`); + console.error('error: You may receive already-processed updates on app restart'); + console.error(`error: Please see ${bugUrl} for more information`); + /* eslint-enable no-console */ + throw err; }); }) .finally(() => { From e94f41b381c8eb1c3b7e64c09e95f519c78bd144 Mon Sep 17 00:00:00 2001 From: GochoMugo Date: Thu, 9 Feb 2017 20:09:31 +0300 Subject: [PATCH 3/4] src/polling: Emit 'error' instead --- doc/usage.md | 1 + src/telegramPolling.js | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/doc/usage.md b/doc/usage.md index 3a17700f..987855be 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -36,6 +36,7 @@ that emits the following events: 1. `pre_checkout_query`: Received a new incoming pre-checkout query 1. `polling_error`: Error occurred during polling. See [polling errors](#polling-errors). 1. `webhook_error`: Error occurred handling a webhook request. See [webhook errors](#webhook-errors). +1. `error`: Unexpected error occurred, usually fatal! **Tip:** Its much better to listen a specific event rather than on `message` in order to stay safe from the content. diff --git a/src/telegramPolling.js b/src/telegramPolling.js index 8df5d1bb..711a62f9 100644 --- a/src/telegramPolling.js +++ b/src/telegramPolling.js @@ -1,3 +1,4 @@ +const errors = require('./errors'); const debug = require('debug')('node-telegram-bot-api'); const deprecate = require('depd')('node-telegram-bot-api'); const ANOTHER_WEB_HOOK_USED = 409; @@ -105,13 +106,24 @@ class TelegramBotPolling { updates.forEach(update => { this.options.params.offset = update.update_id + 1; debug('updated offset: %s', this.options.params.offset); - this.bot.processUpdate(update); + try { + this.bot.processUpdate(update); + } catch (err) { + err._processing = true; + throw err; + } }); return null; }) .catch(err => { debug('polling error: %s', err.message); + if (!err._processing) { + return this._error(err); + } + delete err._processing; /* + * An error occured while processing the items, + * i.e. in `this.bot.processUpdate()` above. * We need to mark the already-processed items * to avoid fetching them again once the application * is restarted, or moves to next polling interval @@ -135,15 +147,17 @@ class TelegramBotPolling { * We have to log this to stderr to ensure devops * understands that they may receive already-processed items * on app restart. + * We simply can not rescue this situation, emit "error" + * event, with the hope that the application exits. */ /* eslint-disable no-console */ const bugUrl = 'https://github.com/yagop/node-telegram-bot-api/issues/36#issuecomment-268532067'; console.error('error: Internal handling of The Offset Infinite Loop failed'); - console.error(`error: This was due to error '${requestErr}'`); + console.error(`error: Due to error '${requestErr}'`); console.error('error: You may receive already-processed updates on app restart'); console.error(`error: Please see ${bugUrl} for more information`); /* eslint-enable no-console */ - throw err; + return this.bot.emit('error', new errors.FatalError(err)); }); }) .finally(() => { From 11832973f792eb21dd4e1c5affc40adea53ec966 Mon Sep 17 00:00:00 2001 From: GochoMugo Date: Tue, 5 Dec 2017 14:55:39 +0300 Subject: [PATCH 4/4] src,doc: Minor fix, Update changelog --- CHANGELOG.md | 1 + src/telegramPolling.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08deb09f..1cb03919 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Changed: Fixed: +1. (#265) Fix the offset infinite loop bug (#36) (by @GochoMugo) 1. Fix game example (by @MCSH) diff --git a/src/telegramPolling.js b/src/telegramPolling.js index 711a62f9..d8674041 100644 --- a/src/telegramPolling.js +++ b/src/telegramPolling.js @@ -87,7 +87,7 @@ class TelegramBotPolling { */ _error(error) { if (!this.bot.listeners('polling_error').length) { - return console.error('error: [polling_error] %j', err); // eslint-disable-line no-console + return console.error('error: [polling_error] %j', error); // eslint-disable-line no-console } return this.bot.emit('polling_error', error); }