From b9f23b00003fc9283fc15d5ca93a95cee35d99d9 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 11 May 2023 12:53:21 -0300 Subject: [PATCH] fix: app status inconsistencies when running multiple instances in a cluster (#29180) --- .../app/apps/server/bridges/activation.ts | 15 +++++++--- .../ee/server/apps/communication/rest.ts | 4 +++ .../server/apps/communication/websockets.ts | 2 +- apps/meteor/package.json | 2 +- .../server/services/apps-engine/service.ts | 30 ++++++++++++++----- yarn.lock | 12 ++++---- 6 files changed, 45 insertions(+), 20 deletions(-) diff --git a/apps/meteor/app/apps/server/bridges/activation.ts b/apps/meteor/app/apps/server/bridges/activation.ts index dce9d0011ec..79b9511b80e 100644 --- a/apps/meteor/app/apps/server/bridges/activation.ts +++ b/apps/meteor/app/apps/server/bridges/activation.ts @@ -11,12 +11,19 @@ export class AppActivationBridge extends ActivationBridge { super(); } - protected async appAdded(app: ProxiedApp): Promise { - await this.orch.getNotifier().appAdded(app.getID()); + protected async appAdded(_app: ProxiedApp): Promise { + // await this.orch.getNotifier().appAdded(app.getID()); + + // Calls made via AppActivationBridge should NOT go through + // View https://github.com/RocketChat/Rocket.Chat/pull/29180 for details + return undefined; } - protected async appUpdated(app: ProxiedApp): Promise { - await this.orch.getNotifier().appUpdated(app.getID()); + protected async appUpdated(_app: ProxiedApp): Promise { + // Calls made via AppActivationBridge should NOT go through + // View https://github.com/RocketChat/Rocket.Chat/pull/29180 for details + // await this.orch.getNotifier().appUpdated(app.getID()); + return undefined; } protected async appRemoved(app: ProxiedApp): Promise { diff --git a/apps/meteor/ee/server/apps/communication/rest.ts b/apps/meteor/ee/server/apps/communication/rest.ts index ccbd2cbf81a..1ce0c9590cc 100644 --- a/apps/meteor/ee/server/apps/communication/rest.ts +++ b/apps/meteor/ee/server/apps/communication/rest.ts @@ -426,6 +426,8 @@ export class AppsRestApi { info.status = success ? AppStatus.AUTO_ENABLED : info.status; } + void orchestrator.getNotifier().appAdded(info.id); + return API.v1.success({ app: info, implemented: aff.getImplementedInferfaces(), @@ -769,6 +771,8 @@ export class AppsRestApi { void notifyAppInstall(orchestrator.getMarketplaceUrl() as string, 'update', info); + void orchestrator.getNotifier().appUpdated(info.id); + return API.v1.success({ app: info, implemented: aff.getImplementedInferfaces(), diff --git a/apps/meteor/ee/server/apps/communication/websockets.ts b/apps/meteor/ee/server/apps/communication/websockets.ts index 58df570e43f..8e027008c26 100644 --- a/apps/meteor/ee/server/apps/communication/websockets.ts +++ b/apps/meteor/ee/server/apps/communication/websockets.ts @@ -40,7 +40,7 @@ export class AppServerListener { } async onAppAdded(appId: string): Promise { - await (this.orch.getManager()! as any).loadOne(appId); // TO-DO: fix type + await (this.orch.getManager()! as any).addLocal(appId); // TO-DO: fix type this.clientStreamer.emitWithoutBroadcast(AppEvents.APP_ADDED, appId); } diff --git a/apps/meteor/package.json b/apps/meteor/package.json index 10834d45325..d8ca4b5bda6 100644 --- a/apps/meteor/package.json +++ b/apps/meteor/package.json @@ -216,7 +216,7 @@ "@rocket.chat/account-utils": "workspace:^", "@rocket.chat/agenda": "workspace:^", "@rocket.chat/api-client": "workspace:^", - "@rocket.chat/apps-engine": "1.39.0-alpha.212", + "@rocket.chat/apps-engine": "1.39.0-alpha.219", "@rocket.chat/base64": "workspace:^", "@rocket.chat/cas-validate": "workspace:^", "@rocket.chat/core-services": "workspace:^", diff --git a/apps/meteor/server/services/apps-engine/service.ts b/apps/meteor/server/services/apps-engine/service.ts index 0c481c16845..4253c9d9efd 100644 --- a/apps/meteor/server/services/apps-engine/service.ts +++ b/apps/meteor/server/services/apps-engine/service.ts @@ -25,19 +25,23 @@ export class AppsEngineService extends ServiceClassInternal implements IAppsEngi }); this.onEvent('apps.added', async (appId: string): Promise => { + Apps.getRocketChatLogger().debug(`"apps.added" event received for app "${appId}"`); // if the app already exists in this instance, don't load it again const app = Apps.getManager()?.getOneById(appId); if (app) { + Apps.getRocketChatLogger().info(`"apps.added" event received for app "${appId}", but it already exists in this instance`); return; } - await (Apps.getManager() as any)?.loadOne(appId); + await Apps.getManager()?.addLocal(appId); }); this.onEvent('apps.removed', async (appId: string): Promise => { + Apps.getRocketChatLogger().debug(`"apps.removed" event received for app "${appId}"`); const app = Apps.getManager()?.getOneById(appId); if (!app) { + Apps.getRocketChatLogger().info(`"apps.removed" event received for app "${appId}", but it couldn't be found in this instance`); return; } @@ -45,8 +49,10 @@ export class AppsEngineService extends ServiceClassInternal implements IAppsEngi }); this.onEvent('apps.updated', async (appId: string): Promise => { + Apps.getRocketChatLogger().debug(`"apps.updated" event received for app "${appId}"`); const storageItem = await Apps.getStorage()?.retrieveOne(appId); if (!storageItem) { + Apps.getRocketChatLogger().info(`"apps.updated" event received for app "${appId}", but it couldn't be found in the storage`); return; } @@ -59,8 +65,15 @@ export class AppsEngineService extends ServiceClassInternal implements IAppsEngi }); this.onEvent('apps.statusUpdate', async (appId: string, status: AppStatus): Promise => { + Apps.getRocketChatLogger().debug(`"apps.statusUpdate" event received for app "${appId}" with status "${status}"`); const app = Apps.getManager()?.getOneById(appId); - if (!app || app.getStatus() === status) { + if (!app) { + Apps.getRocketChatLogger().info(`"apps.statusUpdate" event received for app "${appId}", but it couldn't be found in this instance`); + return; + } + + if (app.getStatus() === status) { + Apps.getRocketChatLogger().info(`"apps.statusUpdate" event received for app "${appId}", but the status is the same`); return; } @@ -72,21 +85,22 @@ export class AppsEngineService extends ServiceClassInternal implements IAppsEngi }); this.onEvent('apps.settingUpdated', async (appId: string, setting: ISetting & { id: string }): Promise => { + Apps.getRocketChatLogger().debug(`"apps.settingUpdated" event received for app "${appId}"`, { setting }); const app = Apps.getManager()?.getOneById(appId); const oldSetting = app?.getStorageItem().settings[setting.id].value; // avoid updating the setting if the value is the same, // which caused an infinite loop if (oldSetting === setting.value) { + Apps.getRocketChatLogger().info( + `"apps.settingUpdated" event received for setting ${setting.id} of app "${appId}", but the setting value is the same`, + ); return; } - const appManager = Apps.getManager(); - if (!appManager) { - return; - } - - await appManager.getSettingsManager().updateAppSetting(appId, setting as any); + await Apps.getManager() + ?.getSettingsManager() + .updateAppSetting(appId, setting as any); }); } diff --git a/yarn.lock b/yarn.lock index b5795b848c7..807767b138c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6226,9 +6226,9 @@ __metadata: languageName: node linkType: hard -"@rocket.chat/apps-engine@npm:1.39.0-alpha.212": - version: 1.39.0-alpha.212 - resolution: "@rocket.chat/apps-engine@npm:1.39.0-alpha.212" +"@rocket.chat/apps-engine@npm:1.39.0-alpha.219": + version: 1.39.0-alpha.219 + resolution: "@rocket.chat/apps-engine@npm:1.39.0-alpha.219" dependencies: adm-zip: ^0.5.9 cryptiles: ^4.1.3 @@ -6240,7 +6240,7 @@ __metadata: vm2: ^3.9.17 peerDependencies: "@rocket.chat/ui-kit": "*" - checksum: 2f43ece57b8f31c3a395b42f3b764de557ccd1058a77c6f8bfff4b9f4fd280f1a00bb5efb78e04507a6bc9dbdd76c103e1e51253422a4f082904bcc9a8a2483a + checksum: db924a4b8d8efb592e5c09f1e938ab330bb10fdfaf4c15a153e9ea749a28371e85bee55686a4f82d61355ec71bd23e43bb9b9dc061f1384330ea27c1688c9cb1 languageName: node linkType: hard @@ -6875,7 +6875,7 @@ __metadata: "@rocket.chat/account-utils": "workspace:^" "@rocket.chat/agenda": "workspace:^" "@rocket.chat/api-client": "workspace:^" - "@rocket.chat/apps-engine": 1.39.0-alpha.212 + "@rocket.chat/apps-engine": 1.39.0-alpha.219 "@rocket.chat/base64": "workspace:^" "@rocket.chat/cas-validate": "workspace:^" "@rocket.chat/core-services": "workspace:^" @@ -24481,7 +24481,7 @@ __metadata: resolution: "lamejs@https://github.com/zhuker/lamejs.git#commit=582bbba6a12f981b984d8fb9e1874499fed85675" dependencies: use-strict: 1.0.1 - checksum: fa829e0c170a65573e653b4d908a44aaf06a50e1bbade3b1217a300a03ccd59a537e294e2d924a584f9d70c7726a12d4c3af9c675436d48d08be5fb94b5eb400 + checksum: ed7f6f1c9629b53c17023eb04b4fc5a222e9c34fcb4a2f61214488fc64e5cfea825e4588d959c5fb20f3a91f0120103fa60307dd43df995d498ff5ddb6200cd9 languageName: node linkType: hard