From 1ce57b11cdcc6488b0fdd505410b17592c2a1e92 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Wed, 25 Mar 2020 22:09:48 -0700 Subject: [PATCH 1/3] added processBeforeResponse flag to ExpressReceiver to better support Faas --- src/App.ts | 3 ++ src/ExpressReceiver.ts | 63 +++++++++++++++++++++++++++++++----------- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/App.ts b/src/App.ts index 8f1f837f2..23c484871 100644 --- a/src/App.ts +++ b/src/App.ts @@ -74,6 +74,7 @@ export interface AppOptions { logLevel?: LogLevel; ignoreSelf?: boolean; clientOptions?: Pick; + processBeforeResponse?: boolean; } export { LogLevel, Logger } from '@slack/logger'; @@ -184,6 +185,7 @@ export default class App { logLevel = undefined, ignoreSelf = true, clientOptions = undefined, + processBeforeResponse = false, }: AppOptions = {}) { if (typeof logger === 'undefined') { @@ -249,6 +251,7 @@ export default class App { this.receiver = new ExpressReceiver({ signingSecret, endpoints, + processBeforeResponse, logger: this.logger, }); } diff --git a/src/ExpressReceiver.ts b/src/ExpressReceiver.ts index 19a37ea7c..514647152 100644 --- a/src/ExpressReceiver.ts +++ b/src/ExpressReceiver.ts @@ -17,6 +17,7 @@ export interface ExpressReceiverOptions { endpoints?: string | { [endpointType: string]: string; }; + processBeforeResponse?: boolean; } /** @@ -30,11 +31,13 @@ export default class ExpressReceiver implements Receiver { private server: Server; private bolt: App | undefined; private logger: Logger; + private processBeforeResponse: boolean; constructor({ signingSecret = '', logger = new ConsoleLogger(), endpoints = { events: '/slack/events' }, + processBeforeResponse = false, }: ExpressReceiverOptions) { this.app = express(); // TODO: what about starting an https server instead of http? what about other options to create the server? @@ -47,6 +50,7 @@ export default class ExpressReceiver implements Receiver { this.requestHandler.bind(this), ]; + this.processBeforeResponse = processBeforeResponse; this.logger = logger; const endpointList: string[] = typeof endpoints === 'string' ? [endpoints] : Object.values(endpoints); for (const endpoint of endpointList) { @@ -64,25 +68,52 @@ export default class ExpressReceiver implements Receiver { // tslint:disable-next-line: align }, 3001); - const event: ReceiverEvent = { - body: req.body, - ack: async (response: any): Promise => { - if (isAcknowledged) { - throw new ReceiverMultipleAckError(); - } - isAcknowledged = true; - if (!response) { - res.send(''); - } else if (typeof response === 'string') { - res.send(response); - } else { - res.json(response); - } - }, - }; + let event: ReceiverEvent; + let storedResponse = undefined; + if (this.processBeforeResponse) { + event = { + body: req.body, + ack: async (response): Promise => { + if (isAcknowledged) { + throw new ReceiverMultipleAckError(); + } + isAcknowledged = true; + if (!response) { + res.send(''); + } else { + storedResponse = response; + } + }, + }; + + } else { + event = { + body: req.body, + ack: async (response): Promise => { + if (isAcknowledged) { + throw new ReceiverMultipleAckError(); + } + isAcknowledged = true; + if (!response) { + res.send(''); + } else if (typeof response === 'string') { + res.send(response); + } else { + res.json(response); + } + }, + }; + } try { await this.bolt?.processEvent(event); + if (storedResponse !== undefined) { + if (typeof storedResponse === 'string') { + res.send(storedResponse); + } else { + res.json(storedResponse); + } + } } catch (err) { res.send(500); throw err; From 8a9abc232c60c9b304b20c6efb5d609d0b562406 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Thu, 26 Mar 2020 10:51:43 -0700 Subject: [PATCH 2/3] updated based on feedback --- src/App.ts | 2 +- src/ExpressReceiver.ts | 37 +++++++++++++------------------------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/App.ts b/src/App.ts index 23c484871..58b379ae0 100644 --- a/src/App.ts +++ b/src/App.ts @@ -62,6 +62,7 @@ const packageJson = require('../package.json'); // tslint:disable-line:no-requir export interface AppOptions { signingSecret?: ExpressReceiverOptions['signingSecret']; endpoints?: ExpressReceiverOptions['endpoints']; + processBeforeResponse?: ExpressReceiverOptions['processBeforeResponse']; agent?: Agent; clientTls?: Pick; convoStore?: ConversationStore | false; @@ -74,7 +75,6 @@ export interface AppOptions { logLevel?: LogLevel; ignoreSelf?: boolean; clientOptions?: Pick; - processBeforeResponse?: boolean; } export { LogLevel, Logger } from '@slack/logger'; diff --git a/src/ExpressReceiver.ts b/src/ExpressReceiver.ts index 514647152..f125bfafa 100644 --- a/src/ExpressReceiver.ts +++ b/src/ExpressReceiver.ts @@ -68,32 +68,21 @@ export default class ExpressReceiver implements Receiver { // tslint:disable-next-line: align }, 3001); - let event: ReceiverEvent; let storedResponse = undefined; - if (this.processBeforeResponse) { - event = { - body: req.body, - ack: async (response): Promise => { - if (isAcknowledged) { - throw new ReceiverMultipleAckError(); - } - isAcknowledged = true; + const event: ReceiverEvent = { + body: req.body, + ack: async (response): Promise => { + if (isAcknowledged) { + throw new ReceiverMultipleAckError(); + } + isAcknowledged = true; + if (this.processBeforeResponse) { if (!response) { - res.send(''); + storedResponse = ''; } else { storedResponse = response; } - }, - }; - - } else { - event = { - body: req.body, - ack: async (response): Promise => { - if (isAcknowledged) { - throw new ReceiverMultipleAckError(); - } - isAcknowledged = true; + } else { if (!response) { res.send(''); } else if (typeof response === 'string') { @@ -101,9 +90,9 @@ export default class ExpressReceiver implements Receiver { } else { res.json(response); } - }, - }; - } + } + }, + }; try { await this.bolt?.processEvent(event); From 0f9c34215bf08a64c12db1eb3c9300c9323ee1f5 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Sun, 29 Mar 2020 22:00:42 -0700 Subject: [PATCH 3/3] updated tests --- src/ExpressReceiver.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ExpressReceiver.spec.ts b/src/ExpressReceiver.spec.ts index e25deb8bd..327474fcf 100644 --- a/src/ExpressReceiver.spec.ts +++ b/src/ExpressReceiver.spec.ts @@ -40,6 +40,7 @@ describe('ExpressReceiver', () => { signingSecret: 'my-secret', logger: noopLogger, endpoints: { events: '/custom-endpoint' }, + processBeforeResponse: true, }); assert.isNotNull(receiver); }); @@ -58,7 +59,7 @@ describe('ExpressReceiver', () => { }); describe('built-in middleware', () => { - describe('ssl_check requset handler', () => { + describe('ssl_check request handler', () => { it('should handle valid requests', async () => { // Arrange // tslint:disable-next-line: no-object-literal-type-assertion