diff --git a/.eslintignore b/.eslintignore index ddc583d55..618271e60 100644 --- a/.eslintignore +++ b/.eslintignore @@ -733,6 +733,9 @@ packages/app-desktop/services/commands/stateToWhenClauseContext.js.map packages/app-desktop/services/commands/types.d.ts packages/app-desktop/services/commands/types.js packages/app-desktop/services/commands/types.js.map +packages/app-desktop/services/plugins/BackOffHandler.d.ts +packages/app-desktop/services/plugins/BackOffHandler.js +packages/app-desktop/services/plugins/BackOffHandler.js.map packages/app-desktop/services/plugins/PlatformImplementation.d.ts packages/app-desktop/services/plugins/PlatformImplementation.js packages/app-desktop/services/plugins/PlatformImplementation.js.map diff --git a/.gitignore b/.gitignore index 9b174f0c7..5fb8565d4 100644 --- a/.gitignore +++ b/.gitignore @@ -723,6 +723,9 @@ packages/app-desktop/services/commands/stateToWhenClauseContext.js.map packages/app-desktop/services/commands/types.d.ts packages/app-desktop/services/commands/types.js packages/app-desktop/services/commands/types.js.map +packages/app-desktop/services/plugins/BackOffHandler.d.ts +packages/app-desktop/services/plugins/BackOffHandler.js +packages/app-desktop/services/plugins/BackOffHandler.js.map packages/app-desktop/services/plugins/PlatformImplementation.d.ts packages/app-desktop/services/plugins/PlatformImplementation.js packages/app-desktop/services/plugins/PlatformImplementation.js.map diff --git a/packages/app-desktop/services/plugins/BackOffHandler.ts b/packages/app-desktop/services/plugins/BackOffHandler.ts new file mode 100644 index 000000000..b3d9d1e03 --- /dev/null +++ b/packages/app-desktop/services/plugins/BackOffHandler.ts @@ -0,0 +1,64 @@ +import Logger from '@joplin/lib/Logger'; +import time from '@joplin/lib/time'; + +const logger = Logger.create('BackOffHandler'); + +// This handler performs two checks: +// +// 1. If the plugin makes many API calls one after the other, a delay is going +// to be applied before responding. The delay is set using backOffIntervals_. +// When a plugin needs to be throttled that way a warning is displayed so +// that the author gets an opportunity to fix it. +// +// 2. If the plugin makes many simultaneous calls (over 100), the handler throws +// an exception to stop the plugin. In that case the plugin will be broken, +// but most plugins will not get this error anyway because call are usually +// made in sequence. It might reveal a bug though - for example if the plugin +// makes a call every 1 second, but does not wait for the response (or assume +// the response will come in less than one second). In that case, the back +// off intervals combined with the incorrect code will make the plugin fail. + +export default class BackOffHandler { + + private backOffIntervals_ = Array(100).fill(0).concat([0, 1, 1, 2, 3, 5, 8]); + private lastRequestTime_ = 0; + private pluginId_: string; + private resetBackOffInterval_ = (this.backOffIntervals_[this.backOffIntervals_.length - 1] + 1) * 1000; + private backOffIndex_ = 0; + private waitCount_ = 0; + private maxWaitCount_ = 100; + + public constructor(pluginId: string) { + this.pluginId_ = pluginId; + } + + private backOffInterval() { + const now = Date.now(); + + if (now - this.lastRequestTime_ > this.resetBackOffInterval_) { + this.backOffIndex_ = 0; + } else { + this.backOffIndex_++; + } + + this.lastRequestTime_ = now; + const effectiveIndex = this.backOffIndex_ >= this.backOffIntervals_.length ? this.backOffIntervals_.length - 1 : this.backOffIndex_; + return this.backOffIntervals_[effectiveIndex]; + } + + public async wait(path: string, args: any) { + const interval = this.backOffInterval(); + if (!interval) return; + + this.waitCount_++; + + logger.warn(`Plugin ${this.pluginId_}: Applying a backoff of ${interval} seconds due to frequent plugin API calls. Consider reducing the number of calls, caching the data, or requesting more data per call. API call was: `, path, args, `[Wait count: ${this.waitCount_}]`); + + if (this.waitCount_ > this.maxWaitCount_) throw new Error(`Plugin ${this.pluginId_}: More than ${this.maxWaitCount_} API alls are waiting - aborting. Please consider queuing the API calls in your plugins to reduce the load on the application.`); + + await time.sleep(interval); + + this.waitCount_--; + } + +} diff --git a/packages/app-desktop/services/plugins/PluginRunner.ts b/packages/app-desktop/services/plugins/PluginRunner.ts index 8dfb5274b..9fdd35953 100644 --- a/packages/app-desktop/services/plugins/PluginRunner.ts +++ b/packages/app-desktop/services/plugins/PluginRunner.ts @@ -7,6 +7,7 @@ import Setting from '@joplin/lib/models/Setting'; import { EventHandlers } from '@joplin/lib/services/plugins/utils/mapEventHandlersToIds'; import shim from '@joplin/lib/shim'; import Logger from '@joplin/lib/Logger'; +import BackOffHandler from './BackOffHandler'; const ipcRenderer = require('electron').ipcRenderer; const logger = Logger.create('PluginRunner'); @@ -83,8 +84,9 @@ function mapEventIdsToHandlers(pluginId: string, arg: any) { export default class PluginRunner extends BasePluginRunner { protected eventHandlers_: EventHandlers = {}; + private backOffHandlers_: Record = {}; - constructor() { + public constructor() { super(); this.eventHandler = this.eventHandler.bind(this); @@ -95,7 +97,14 @@ export default class PluginRunner extends BasePluginRunner { return cb(...args); } - async run(plugin: Plugin, pluginApi: Global) { + private backOffHandler(pluginId: string): BackOffHandler { + if (!this.backOffHandlers_[pluginId]) { + this.backOffHandlers_[pluginId] = new BackOffHandler(pluginId); + } + return this.backOffHandlers_[pluginId]; + } + + public async run(plugin: Plugin, pluginApi: Global) { const scriptPath = `${Setting.value('tempDir')}/plugin_${plugin.id}.js`; await shim.fsDriver().writeFile(scriptPath, plugin.scriptText, 'utf8'); @@ -148,6 +157,13 @@ export default class PluginRunner extends BasePluginRunner { const debugMappedArgs = fullPath.includes('setHtml') ? '' : mappedArgs; logger.debug(`Got message (3): ${fullPath}`, debugMappedArgs); + try { + await this.backOffHandler(plugin.id).wait(fullPath, debugMappedArgs); + } catch (error) { + logger.error(error); + return; + } + let result: any = null; let error: any = null; try {