From ba416e165639d204bd2260e3b07045ecd999a529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Thu, 14 Sep 2023 08:19:38 +0100 Subject: [PATCH] fix: use postmessage in slack app addon (#4688) https://linear.app/unleash/issue/2-1392/fix-flaky-unhandled-events - Reverts part of https://github.com/Unleash/unleash/pull/4490 and uses `postMessage` instead. This prevents an error where scheduling the message was flaky and some events ended up not being handled at all (see screenshot below). As a bonus, this simplifies our code and prevents having a delay. It seems like this method still works with the channel name instead of needing an id, which was the main motivation towards the changes in the aforementioned PR; - Removes `FEATURE_UPDATED` from the event options for the Slack App addon, as [this event is deprecated](https://docs.getunleash.io/reference/environments#addons); - Adds support for events without a specific project, including a test, similar to https://github.com/Unleash/unleash/pull/4672 - Misc cleanups; ![image](https://github.com/Unleash/unleash/assets/14320932/3fcd085c-a60f-42f6-9739-b600db7a7cee) --- src/lib/addons/slack-app-definition.ts | 2 -- src/lib/addons/slack-app.test.ts | 14 +++++----- src/lib/addons/slack-app.ts | 10 +++---- src/lib/services/addon-service.test.ts | 36 ++++++++++++++++++++++++++ src/lib/services/addon-service.ts | 1 + 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/lib/addons/slack-app-definition.ts b/src/lib/addons/slack-app-definition.ts index d61fe3fc38..039ba38bd6 100644 --- a/src/lib/addons/slack-app-definition.ts +++ b/src/lib/addons/slack-app-definition.ts @@ -1,6 +1,5 @@ import { FEATURE_CREATED, - FEATURE_UPDATED, FEATURE_ARCHIVED, FEATURE_REVIVED, FEATURE_STALE_ON, @@ -70,7 +69,6 @@ const slackAppDefinition: IAddonDefinition = { ], events: [ FEATURE_CREATED, - FEATURE_UPDATED, FEATURE_ARCHIVED, FEATURE_REVIVED, FEATURE_STALE_ON, diff --git a/src/lib/addons/slack-app.test.ts b/src/lib/addons/slack-app.test.ts index fed3a56c32..ffd56e944c 100644 --- a/src/lib/addons/slack-app.test.ts +++ b/src/lib/addons/slack-app.test.ts @@ -4,7 +4,7 @@ import { ChatPostMessageArguments, ErrorCode } from '@slack/web-api'; const slackApiCalls: ChatPostMessageArguments[] = []; -let scheduleMessage = jest.fn().mockImplementation((options) => { +let postMessage = jest.fn().mockImplementation((options) => { slackApiCalls.push(options); return Promise.resolve(); }); @@ -12,7 +12,7 @@ let scheduleMessage = jest.fn().mockImplementation((options) => { jest.mock('@slack/web-api', () => ({ WebClient: jest.fn().mockImplementation(() => ({ chat: { - scheduleMessage, + postMessage, }, on: jest.fn(), })), @@ -60,7 +60,7 @@ describe('SlackAppAddon', () => { beforeEach(() => { jest.useFakeTimers(); slackApiCalls.length = 0; - scheduleMessage.mockClear(); + postMessage.mockClear(); addon = new SlackAppAddon({ getLogger, unleashUrl: 'http://some-url.com', @@ -124,7 +124,7 @@ describe('SlackAppAddon', () => { }); it('should log error when an API call fails', async () => { - scheduleMessage = jest.fn().mockRejectedValue(mockError); + postMessage = jest.fn().mockRejectedValue(mockError); await addon.handleEvent(event, { accessToken }); @@ -134,7 +134,7 @@ describe('SlackAppAddon', () => { ); }); - it('should handle rejections in chat.scheduleMessage', async () => { + it('should handle rejections in chat.postMessage', async () => { const eventWith3Tags: IEvent = { ...event, tags: [ @@ -144,7 +144,7 @@ describe('SlackAppAddon', () => { ], }; - scheduleMessage = jest + postMessage = jest .fn() .mockResolvedValueOnce({ ok: true }) .mockResolvedValueOnce({ ok: true }) @@ -152,7 +152,7 @@ describe('SlackAppAddon', () => { await addon.handleEvent(eventWith3Tags, { accessToken }); - expect(scheduleMessage).toHaveBeenCalledTimes(3); + expect(postMessage).toHaveBeenCalledTimes(3); expect(loggerMock.warn).toHaveBeenCalledWith( `Error handling event ${FEATURE_ENVIRONMENT_ENABLED}. A platform error occurred: Platform error message`, expect.any(Object), diff --git a/src/lib/addons/slack-app.ts b/src/lib/addons/slack-app.ts index 47f901de8c..5727ff4d6d 100644 --- a/src/lib/addons/slack-app.ts +++ b/src/lib/addons/slack-app.ts @@ -12,7 +12,6 @@ import Addon from './addon'; import slackAppDefinition from './slack-app-definition'; import { IAddonConfig } from '../types/model'; -const SCHEDULE_MESSAGE_DELAY_IN_SECONDS = 10; import { FeatureEventFormatter, FeatureEventFormatterMd, @@ -52,9 +51,11 @@ export default class SlackAppAddon extends Addon { this.logger.warn('No access token provided.'); return; } - let postToDefault = + + const postToDefault = alwaysPostToDefault === 'true' || alwaysPostToDefault === 'yes'; this.logger.debug(`Post to default was set to ${postToDefault}`); + const taggedChannels = this.findTaggedChannels(event); let eventChannels: string[]; if (postToDefault) { @@ -89,9 +90,7 @@ export default class SlackAppAddon extends Addon { const text = this.msgFormatter.format(event); const url = this.msgFormatter.featureLink(event); const requests = eventChannels.map((name) => { - const now = Math.floor(new Date().getTime() / 1000); - const postAt = now + SCHEDULE_MESSAGE_DELAY_IN_SECONDS; - return this.slackClient!.chat.scheduleMessage({ + return this.slackClient!.chat.postMessage({ channel: name, text, blocks: [ @@ -118,7 +117,6 @@ export default class SlackAppAddon extends Addon { ], }, ], - post_at: postAt, }); }); diff --git a/src/lib/services/addon-service.test.ts b/src/lib/services/addon-service.test.ts index c00050613c..590114e607 100644 --- a/src/lib/services/addon-service.test.ts +++ b/src/lib/services/addon-service.test.ts @@ -329,6 +329,42 @@ test('should not filter out global events (no specific environment) even if addo expect(events[0].event.data.name).toBe('some-toggle'); }); +test('should not filter out global events (no specific project) even if addon is setup to filter for projects', async () => { + const { addonService, stores } = getSetup(); + const filteredProject = 'filtered'; + const config = { + provider: 'simple', + enabled: true, + events: [FEATURE_CREATED], + projects: [filteredProject], + environments: [], + description: '', + parameters: { + url: 'http://localhost:wh', + }, + }; + + const globalEventWithNoProject = { + type: FEATURE_CREATED, + createdBy: 'some@user.com', + data: { + name: 'some-toggle', + enabled: false, + strategies: [{ name: 'default' }], + }, + }; + + await addonService.createAddon(config, 'me@mail.com'); + await stores.eventStore.store(globalEventWithNoProject); + const simpleProvider = addonService.addonProviders.simple; + // @ts-expect-error + const events = simpleProvider.getEvents(); + + expect(events.length).toBe(1); + expect(events[0].event.type).toBe(FEATURE_CREATED); + expect(events[0].event.data.name).toBe('some-toggle'); +}); + test('should support wildcard option for filtering addons', async () => { const { addonService, stores } = getSetup(); const desiredProjects = ['desired', 'desired2']; diff --git a/src/lib/services/addon-service.ts b/src/lib/services/addon-service.ts index a7a25822ff..3105d321a9 100644 --- a/src/lib/services/addon-service.ts +++ b/src/lib/services/addon-service.ts @@ -114,6 +114,7 @@ export default class AddonService { .filter((addon) => addon.events.includes(eventName)) .filter( (addon) => + !event.project || !addon.projects || addon.projects.length == 0 || addon.projects[0] === WILDCARD_OPTION ||