From 0d662d089d5483f44ebcf36a87904c920e4e9eee Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Wed, 1 Apr 2020 19:12:52 +0900 Subject: [PATCH 1/2] Add failing tests --- src/App.spec.ts | 83 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/App.spec.ts b/src/App.spec.ts index aee109e35..8c5d5487f 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -1257,6 +1257,89 @@ describe('App', () => { assert.equal(fakeErrorHandler.callCount, dummyReceiverEvents.length); }); }); + + describe('processBeforeResponse', () => { + + it('should call ack() after event listener completion', async () => { + // Arrange + const App = await importApp(overrides); // tslint:disable-line:variable-name + const fakeLogger = createFakeLogger(); + const app = new App({ + logger: fakeLogger, + receiver: fakeReceiver, + authorize: sinon.fake.resolves(dummyAuthorizationResult), + processBeforeResponse: true, + }); + + let acknowledged = false; + + app.event('app_home_opened', async ({}) => { + await new Promise(resolve => setTimeout(resolve, 200)); + assert.isFalse(acknowledged); + }); + + const event = { + body: { + type: 'event_callback', + token: 'XXYYZZ', + team_id: 'TXXXXXXXX', + api_app_id: 'AXXXXXXXXX', + event: { + type: 'app_home_opened', + event_ts: '1234567890.123456', + user: 'UXXXXXXX1', + text: 'hello friends!', + tab: 'home', + view: {}, + }, + }, + respond: noop, + ack: async (_res: any) => { + acknowledged = true; + }, + }; + + // Act + await fakeReceiver.sendEvent(event); + + // Assert + assert.isTrue(acknowledged); + }); + + it('should call ack() after command listener completion', async () => { + // Arrange + const App = await importApp(overrides); // tslint:disable-line:variable-name + const fakeLogger = createFakeLogger(); + const app = new App({ + logger: fakeLogger, + receiver: fakeReceiver, + authorize: sinon.fake.resolves(dummyAuthorizationResult), + processBeforeResponse: true, + }); + + let acknowledged = false; + + app.command('/hello', async ({ ack }) => { + assert.isFalse(acknowledged); + await ack(); + }); + + const event = { + ...baseEvent, + body: { + command: '/hello', + }, + ack: async (_res: any) => { + acknowledged = true; + }, + }; + + // Act + await fakeReceiver.sendEvent(event); + // Assert + assert.isTrue(acknowledged); + }); + }); }); }); }); From c2bb0221d7f456b64a0afff9c873b71a212a0158 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Wed, 1 Apr 2020 19:23:53 +0900 Subject: [PATCH 2/2] Re-fix #395 by adjusting the processBeforeResponse option --- src/App.ts | 14 ++++++++++++-- src/ExpressReceiver.ts | 22 +++++++++++++++++----- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/App.ts b/src/App.ts index 58b379ae0..fe8052abf 100644 --- a/src/App.ts +++ b/src/App.ts @@ -154,6 +154,9 @@ export default class App { /** Receiver - ingests events from the Slack platform */ private receiver: Receiver; + /** Always call ack() after event listener completion if true */ + private processBeforeResponse: boolean = false; + /** Logger */ private logger: Logger; @@ -256,6 +259,7 @@ export default class App { }); } } + this.processBeforeResponse = processBeforeResponse; // Conditionally use a global middleware that ignores events (including messages) that are sent from this app if (ignoreSelf) { @@ -580,8 +584,11 @@ export default class App { if (type !== IncomingEventType.Event) { listenerArgs.ack = ack; } else { - // Events API requests are acknowledged right away, since there's no data expected - await ack(); + // If processBeforeResponse is true, ack() call should be after the listener completion + if (!this.processBeforeResponse) { + // Events API requests are acknowledged right away, since there's no data expected + await ack(); + } } // Get the client arg @@ -635,6 +642,9 @@ export default class App { } else if (rejectedListenerResults.length > 1) { throw new MultipleListenerError(rejectedListenerResults.map(rlr => rlr.reason)); } + if (this.processBeforeResponse && type === IncomingEventType.Event) { + await ack(); + } }, ); } catch (error) { diff --git a/src/ExpressReceiver.ts b/src/ExpressReceiver.ts index f125bfafa..ea55d1560 100644 --- a/src/ExpressReceiver.ts +++ b/src/ExpressReceiver.ts @@ -96,12 +96,24 @@ export default class ExpressReceiver implements Receiver { try { await this.bolt?.processEvent(event); - if (storedResponse !== undefined) { - if (typeof storedResponse === 'string') { - res.send(storedResponse); - } else { - res.json(storedResponse); + + if (this.processBeforeResponse) { + let spentMillis = 0; + const millisForTimeout = 10; + // Wait here until the isAcknowledged is marked as true or 3 seconds have passed + while (!isAcknowledged && spentMillis < 3000) { + await new Promise(resolve => setTimeout(resolve, millisForTimeout)); + spentMillis += millisForTimeout; } + if (isAcknowledged) { + this.logger.debug(`The listener execution completed in ${spentMillis} millis`); + if (typeof storedResponse === 'string') { + res.send(storedResponse); + } else { + res.json(storedResponse); + } + this.logger.debug('Acknowledged after the listener completion'); + } // Otherwise, this Bolt app never responds to this request and above setTimeout outputs an error message } } catch (err) { res.send(500);