From 9166b68af7acba4db2580cf8df7a77410e21639b Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 24 Mar 2020 15:30:07 -0700 Subject: [PATCH 1/3] updated based on feedback from #380/#381 --- README.md | 3 +- docs/_advanced/middleware_global.md | 2 +- docs/_advanced/receiver.md | 4 +- docs/_basic/listening_responding_shortcuts.md | 2 +- src/App.spec.ts | 1 - src/ExpressReceiver.ts | 54 ++++++++----------- src/errors.spec.ts | 4 +- src/errors.ts | 9 +--- 8 files changed, 31 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 1875085a3..e1688124d 100644 --- a/README.md +++ b/README.md @@ -256,7 +256,6 @@ async function authWithAcme({ payload, context, say, next }) { // When the user lookup is successful, add the user details to the context context.user = user; } catch (error) { - // middleware/listeners continue if (error.message === 'Not Found') { // In the real world, you would need to check if the say function was defined, falling back to the respond // function if not, and then falling back to only logging the error as a last resort. @@ -327,7 +326,7 @@ In general, a middleware can run both before and after the remaining middleware How you use `next` can have four different effects: -* **To both preprocess and post-process events** - You can choose to do work going _before_ listener functions by putting code +* **To both preprocess and post-process events** - You can choose to do work both _before_ listener functions by putting code before `await next()` and _after_ by putting code after `await next()`. `await next()` passes control down the middleware stack in the order it was defined, then back up it in reverse order. diff --git a/docs/_advanced/middleware_global.md b/docs/_advanced/middleware_global.md index 02b6af852..b4b7ecf30 100644 --- a/docs/_advanced/middleware_global.md +++ b/docs/_advanced/middleware_global.md @@ -6,7 +6,7 @@ order: 4 ---
-Global middleware is run for all incoming events before any listener middleware. You can add any number of global middleware to your app by utilizing `app.use(fn({payload,...,next}))`. +Global middleware is run for all incoming events before any listener middleware. You can add any number of global middleware to your app by utilizing `app.use(fn)`. The middleware function `fn` is called with the same arguments as listeners and an additional `next` function. Both global and listener middleware must call `await next()` to pass control of the execution chain to the next middleware, or call `throw` to pass an error back up the previously-executed middleware chain. diff --git a/docs/_advanced/receiver.md b/docs/_advanced/receiver.md index 87e189f0c..eb9af501a 100644 --- a/docs/_advanced/receiver.md +++ b/docs/_advanced/receiver.md @@ -15,11 +15,11 @@ A receiver is responsible for handling and parsing any incoming events from Slac | `stop()` | None | `Promise` | `init()` is called after Bolt for JavaScript app is created. This method gives the receiver a reference to an `App` to store so that it can call: -* `await app.processEvent(event)` whenever your app receives an event from Slack. It will reject if there is an unhandled error. +* `await app.processEvent(event)` whenever your app receives an event from Slack. It will throw if there is an unhandled error. To use a custom receiver, you can pass it into the constructor when initializing your Bolt for JavaScript app. Here is what a basic custom receiver might look like. -For a more in-depth look at a receiver, [read the source code for the built-in Express receiver](https://github.com/slackapi/bolt/blob/master/src/ExpressReceiver.ts) +For a more in-depth look at a receiver, [read the source code for the built-in `ExpressReceiver`](https://github.com/slackapi/bolt/blob/master/src/ExpressReceiver.ts)
```javascript diff --git a/docs/_basic/listening_responding_shortcuts.md b/docs/_basic/listening_responding_shortcuts.md index c0db2d905..5d3227eab 100644 --- a/docs/_basic/listening_responding_shortcuts.md +++ b/docs/_basic/listening_responding_shortcuts.md @@ -82,7 +82,7 @@ app.shortcut('open_modal', async ({ shortcut, ack, context, client }) => { ```javascript // Your middleware will only be called when the callback_id matches 'open_modal' AND the type matches 'message_action' - app.shortcut({ callback_id: 'open_modal', type: 'message_action' }, async ({ action, ack, context, client }) => { + app.shortcut({ callback_id: 'open_modal', type: 'message_action' }, async ({ shortcut, ack, context, client }) => { try { // Acknowledge shortcut request await ack(); diff --git a/src/App.spec.ts b/src/App.spec.ts index ad04612ac..e96ef3419 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -440,7 +440,6 @@ describe('App', () => { app.error(async (actualError) => { assert.instanceOf(actualError, UnknownError); assert.equal(actualError.message, error.message); - await delay(); // Make this async to make sure error handlers can be tested }); await fakeReceiver.sendEvent(dummyReceiverEvent); diff --git a/src/ExpressReceiver.ts b/src/ExpressReceiver.ts index 7230a338b..19a37ea7c 100644 --- a/src/ExpressReceiver.ts +++ b/src/ExpressReceiver.ts @@ -1,12 +1,12 @@ import { AnyMiddlewareArgs, Receiver, ReceiverEvent } from './types'; import { createServer, Server } from 'http'; -import express, { Request, Response, Application, RequestHandler, NextFunction } from 'express'; +import express, { Request, Response, Application, RequestHandler } from 'express'; import rawBody from 'raw-body'; import querystring from 'querystring'; import crypto from 'crypto'; import tsscmp from 'tsscmp'; import App from './App'; -import { ReceiverAuthenticityError, ReceiverAckTimeoutError, ReceiverMultipleAckError } from './errors'; +import { ReceiverAuthenticityError, ReceiverMultipleAckError } from './errors'; import { Logger, ConsoleLogger } from '@slack/logger'; // TODO: we throw away the key names for endpoints, so maybe we should use this interface. is it better for migrations? @@ -29,6 +29,7 @@ export default class ExpressReceiver implements Receiver { private server: Server; private bolt: App | undefined; + private logger: Logger; constructor({ signingSecret = '', @@ -36,7 +37,6 @@ export default class ExpressReceiver implements Receiver { endpoints = { events: '/slack/events' }, }: ExpressReceiverOptions) { this.app = express(); - this.app.use(this.errorHandler.bind(this)); // TODO: what about starting an https server instead of http? what about other options to create the server? this.server = createServer(this.app); @@ -47,6 +47,7 @@ export default class ExpressReceiver implements Receiver { this.requestHandler.bind(this), ]; + this.logger = logger; const endpointList: string[] = typeof endpoints === 'string' ? [endpoints] : Object.values(endpoints); for (const endpoint of endpointList) { this.app.post(endpoint, ...expressMiddleware); @@ -54,33 +55,28 @@ export default class ExpressReceiver implements Receiver { } private async requestHandler(req: Request, res: Response): Promise { - let timer: NodeJS.Timeout | undefined = setTimeout( - () => { - this.bolt?.handleError(new ReceiverAckTimeoutError( - 'An incoming event was not acknowledged before the timeout. ' + - 'Ensure that the ack() argument is called in your listeners.', - )); - timer = undefined; - }, - 2800, - ); + let isAcknowledged = false; + setTimeout(() => { + if (!isAcknowledged) { + this.logger.error('An incoming event was not acknowledged within 3 seconds. ' + + 'Ensure that the ack() argument is called in a listener.'); + } + // tslint:disable-next-line: align + }, 3001); const event: ReceiverEvent = { body: req.body, - ack: async (response): Promise => { - if (timer !== undefined) { - clearTimeout(timer); - timer = undefined; - - if (!response) { - res.send(''); - } else if (typeof response === 'string') { - res.send(response); - } else { - res.json(response); - } + 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 { - this.bolt?.handleError(new ReceiverMultipleAckError()); + res.json(response); } }, }; @@ -129,12 +125,6 @@ export default class ExpressReceiver implements Receiver { }); }); } - - private errorHandler(err: any, _req: Request, _res: Response, next: NextFunction): void { - this.bolt?.handleError(err); - // Forward to express' default error handler (which knows how to print stack traces in development) - next(err); - } } export const respondToSslCheck: RequestHandler = (req, res, next) => { diff --git a/src/errors.spec.ts b/src/errors.spec.ts index b06e6e5ec..ac56a2819 100644 --- a/src/errors.spec.ts +++ b/src/errors.spec.ts @@ -7,8 +7,8 @@ import { AppInitializationError, AuthorizationError, ContextMissingPropertyError, - ReceiverAckTimeoutError, ReceiverAuthenticityError, + ReceiverMultipleAckError, UnknownError, } from './errors'; @@ -19,8 +19,8 @@ describe('Errors', () => { [ErrorCode.AppInitializationError]: new AppInitializationError(), [ErrorCode.AuthorizationError]: new AuthorizationError('auth failed', new Error('auth failed')), [ErrorCode.ContextMissingPropertyError]: new ContextMissingPropertyError('foo', "can't find foo"), - [ErrorCode.ReceiverAckTimeoutError]: new ReceiverAckTimeoutError(), [ErrorCode.ReceiverAuthenticityError]: new ReceiverAuthenticityError(), + [ErrorCode.ReceiverMultipleAckError]: new ReceiverMultipleAckError(), [ErrorCode.UnknownError]: new UnknownError(new Error('It errored')), }; diff --git a/src/errors.ts b/src/errors.ts index f8150e9c6..0e20a75b2 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -8,8 +8,7 @@ export enum ErrorCode { ContextMissingPropertyError = 'slack_bolt_context_missing_property_error', - ReceiverAckTimeoutError = 'slack_bolt_receiver_ack_timeout_error', - ReceiverAckTwiceError = 'slack_bolt_receiver_ack_twice_error', + ReceiverMultipleAckError = 'slack_bolt_receiver_ack_multiple_error', ReceiverAuthenticityError = 'slack_bolt_receiver_authenticity_error', /** @@ -52,12 +51,8 @@ export class ContextMissingPropertyError extends Error implements CodedError { } } -export class ReceiverAckTimeoutError extends Error implements CodedError { - public code = ErrorCode.ReceiverAckTimeoutError; -} - export class ReceiverMultipleAckError extends Error implements CodedError { - public code = ErrorCode.ReceiverAckTimeoutError; + public code = ErrorCode.ReceiverMultipleAckError; constructor() { super("The receiver's `ack` function was called multiple times."); From 456d125cf186580dd9eaae000ff6487016d0056d Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Tue, 24 Mar 2020 10:59:22 +0900 Subject: [PATCH 2/3] Update Japanese document about shortcuts --- docs/_advanced/ja_receiver.md | 7 +- .../ja_ listening_responding_shortcuts.md | 65 --------- .../ja_listening_responding_shortcuts.md | 133 ++++++++++++++++++ docs/_tutorials/ja_getting_started.md | 2 +- 4 files changed, 137 insertions(+), 70 deletions(-) delete mode 100644 docs/_basic/ja_ listening_responding_shortcuts.md create mode 100644 docs/_basic/ja_listening_responding_shortcuts.md diff --git a/docs/_advanced/ja_receiver.md b/docs/_advanced/ja_receiver.md index 18dfabaf1..bb54a3629 100644 --- a/docs/_advanced/ja_receiver.md +++ b/docs/_advanced/ja_receiver.md @@ -14,13 +14,12 @@ order: 8 | `start()` | None | `Promise` | | `stop()` | None | `Promise` | -Bolt アプリでは `init()` が 2 回呼び出されます。 -* `await app.processEvent(event)` は、解析されたすべての着信リクエストを `onIncomingEvent()` にルーティングする必要があります。これは、Bolt アプリでは `this.receiver.on('message', message => this.onIncomingEvent(message))` として呼び出されます。 -* `await app.handleError` は、エラーをグローバルエラーハンドラーにルーティングする必要があります。これは、Bolt アプリでは `this.receiver.on('error', error => this.onGlobalError(error))` として呼び出されます。 +Bolt アプリでは `init()` が 2 回呼び出されます。このメソッドはレシーバーに `App` の参照を付与するため、これにより以下の呼び出しが可能になります。 +* `await app.processEvent(event)` は Slack から送信されてくるイベントを受け取るたびに呼び出されます。ハンドリングされなかったエラーが発生した場合はそれを throw します。 Bolt アプリを初期化するときにカスタムレシーバーをコンストラクタに渡すことで、そのカスタムレシーバーを使用できます。ここで紹介するのは、基本的なカスタムレシーバーです。 -レシーバーについて詳しくは、[組み込み Express レシーバーのソースコード](https://github.com/slackapi/bolt/blob/master/src/ExpressReceiver.ts)をお読みください。 +レシーバーについて詳しくは、[組み込み `ExpressReceiver` のソースコード](https://github.com/slackapi/bolt/blob/master/src/ExpressReceiver.ts)をお読みください。 ```javascript diff --git a/docs/_basic/ja_ listening_responding_shortcuts.md b/docs/_basic/ja_ listening_responding_shortcuts.md deleted file mode 100644 index 2e9581085..000000000 --- a/docs/_basic/ja_ listening_responding_shortcuts.md +++ /dev/null @@ -1,65 +0,0 @@ ---- -title: グローバルショートカットのリスニング -lang: ja-jp -slug: shortcuts -order: 8 ---- - -
-[グローバルショートカット](https://api.slack.com/interactivity/shortcuts/using#global_shortcuts)は、テキスト入力エリアや検索バーから起動できる Slack クライアント内の UI エレメントです。`shortcut()` メソッドを使って、グローバルショートカットのイベントをリスニングすることができます。このメソッドには `callback_id` を文字列または正規表現のデータ型で設定します。 - -グローバルショートカットのイベントは Slack へイベントを受信したことを知らせるために `ack()` メソッドで確認する必要があります。 - -グローバルショートカットのペイロードは、ユーザーの実行アクションの確認のために[モーダルを開く](#creating-modals)などの用途に使用できる `trigger_id` を含んでいます。グローバルショートカットのペイロードは **チャンネル ID は含んでいない** ことに注意してください。もしあなたのアプリがチャンネル ID を知る必要があれば、モーダル内で [`conversations_select`](https://api.slack.com/reference/block-kit/block-elements#conversation_select) エレメントを使用できます。 - -⚠️ [メッセージショートカット](https://api.slack.com/interactivity/shortcuts/using#message_shortcuts)は引き続き[`action()` メソッド](#action-listening)を使用するので注意してください。**Bolt の次のメジャーバージョンからはグローバルショートカットもメッセージショートカットも両方とも `shortcut()` メソッドを使用します。** -
- -```javascript -// open_modal というグローバルショートカットはシンプルなモーダルを開く -app.shortcut('open_modal', async ({ payload, ack, context }) => { - // グローバルショートカットリクエストの確認 - ack(); - try { - // 組み込みの WebClient を使って views.open API メソッドを呼び出す - const result = await app.client.views.open({ - // `context` オブジェクトに保持されたトークンを使用 - token: context.botToken, - trigger_id: payload.trigger_id, - view: { - "type": "modal", - "title": { - "type": "plain_text", - "text": "My App" - }, - "close": { - "type": "plain_text", - "text": "Close" - }, - "blocks": [ - { - "type": "section", - "text": { - "type": "mrkdwn", - "text": "About the simplest modal you could conceive of :smile:\n\nMaybe or ." - } - }, - { - "type": "context", - "elements": [ - { - "type": "mrkdwn", - "text": "Psssst this modal was designed using " - } - ] - } - ] - } - }); - console.log(result); - } - catch (error) { - console.error(error); - } -}); -``` diff --git a/docs/_basic/ja_listening_responding_shortcuts.md b/docs/_basic/ja_listening_responding_shortcuts.md new file mode 100644 index 000000000..0d02fbf6b --- /dev/null +++ b/docs/_basic/ja_listening_responding_shortcuts.md @@ -0,0 +1,133 @@ +--- +title: ショートカットのリスニング +lang: ja-jp +slug: shortcuts +order: 8 +--- + +
+`shortcut()` メソッドは、[グローバルショートカット](https://api.slack.com/interactivity/shortcuts/using#global_shortcuts)と[メッセージショートカット](https://api.slack.com/interactivity/shortcuts/using#message_shortcuts)の両方をサポートします。 + +ショートカットは、テキスト入力エリアや検索バーから起動できる Slack クライアント内の UI エレメントです。グローバルショートカットは、コンポーザーメニューまたは検索メニューから呼び出すことができます。メッセージショートカットは、メッセージのコンテキストメニュー内にあります。`shortcut()` メソッドを使って、これらのショートカットのイベントをリスニングすることができます。このメソッドには `callback_id` を文字列または正規表現のデータ型で設定します。 + +グローバルショートカットのイベントは Slack へイベントを受信したことを知らせるために `ack()` メソッドで確認する必要があります。 + +グローバルショートカットのペイロードは、ユーザーの実行アクションの確認のために[モーダルを開く](#creating-modals)などの用途に使用できる `trigger_id` を含んでいます。 + +⚠️ グローバルショートカットのペイロードは **チャンネル ID は含んでいない** ことに注意してください。もしあなたのアプリがチャンネル ID を知る必要があれば、モーダル内で [`conversations_select`](https://api.slack.com/reference/block-kit/block-elements#conversation_select) エレメントを使用できます。 +メッセージショートカットのペイロードはチャンネル ID を含みます。 +
+ +```javascript +// open_modal というグローバルショートカットはシンプルなモーダルを開く +app.shortcut('open_modal', async ({ shortcut, ack, context }) => { + // グローバルショートカットリクエストの確認 + ack(); + + try { + // 組み込みの WebClient を使って views.open API メソッドを呼び出す + const result = await app.client.views.open({ + // `context` オブジェクトに保持されたトークンを使用 + token: context.botToken, + trigger_id: shortcut.trigger_id, + view: { + "type": "modal", + "title": { + "type": "plain_text", + "text": "My App" + }, + "close": { + "type": "plain_text", + "text": "Close" + }, + "blocks": [ + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "About the simplest modal you could conceive of :smile:\n\nMaybe or ." + } + }, + { + "type": "context", + "elements": [ + { + "type": "mrkdwn", + "text": "Psssst this modal was designed using " + } + ] + } + ] + } + }); + + console.log(result); + } + catch (error) { + console.error(error); + } +}); +``` + +
+ +

制約付きオブジェクトを使用したショートカットのリスニング

+ + +
+ 制約付きオブジェクトを使って `callback_id` や `type` によるリスニングをすることができます。オブジェクト内の制約は文字列型または RegExp オブジェクトを使用できます。 + +
+ + ```javascript + // callback_id が 'open_modal' と一致し type が 'message_action' と一致する場合のみミドルウェアが呼び出される + app.shortcut({ callback_id: 'open_modal', type: 'message_action' }, async ({ shortcut, ack, context, client }) => { + try { + // ショートカットリクエストの確認 + await ack(); + + // 組み込みの WebClient を使って views.open API メソッドを呼び出す + const result = await app.client.views.open({ + // `context` オブジェクトに保持されたトークンを使用 + token: context.botToken, + trigger_id: shortcut.trigger_id, + view: { + type: "modal", + title: { + type: "plain_text", + text: "My App" + }, + close: { + type: "plain_text", + text: "Close" + }, + blocks: [ + { + type: "section", + text: { + type: "mrkdwn", + text: "About the simplest modal you could conceive of :smile:\n\nMaybe or ." + } + }, + { + type: "context", + elements: [ + { + type: "mrkdwn", + text: "Psssst this modal was designed using " + } + ] + } + ] + } + }); + + console.log(result); + } + catch (error) { + console.error(error); + } + }); + ``` + +
\ No newline at end of file diff --git a/docs/_tutorials/ja_getting_started.md b/docs/_tutorials/ja_getting_started.md index 2a137e80c..738ef479d 100644 --- a/docs/_tutorials/ja_getting_started.md +++ b/docs/_tutorials/ja_getting_started.md @@ -191,7 +191,7 @@ app.message('hello', async ({ message, say }) => { ### アクションの送信と応答 -ボタン、選択メニュー、日付ピッカー、ダイアログ、メッセージショートカットなどの機能を使用するには、インタラクティブ性を有効にする必要があります。イベントと同様に、Slack の URL を指定してアクション ( 「ボタン・クリック」など) を送信する必要があります。 +ボタン、選択メニュー、日付ピッカー、ダイアログなどの機能を使用するには、インタラクティブ性を有効にする必要があります。イベントと同様に、Slack の URL を指定してアクション ( 「ボタン・クリック」など) を送信する必要があります。 アプリ設定ページに戻り、左側の **Interactive Components** をクリックします。**Request URL** ボックスがもう 1 つあることがわかります。 From cf5650661c0b1cbc48c122e6efa17e946701fb04 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Wed, 25 Mar 2020 14:51:24 -0700 Subject: [PATCH 3/3] updated test based on feedback --- src/App.spec.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index e96ef3419..8f86b3e8d 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -4,7 +4,7 @@ import sinon, { SinonSpy } from 'sinon'; import { assert } from 'chai'; import { Override, mergeOverrides, createFakeLogger, delay } from './test-helpers'; import rewiremock from 'rewiremock'; -import { ErrorCode, UnknownError } from './errors'; +import { ErrorCode, UnknownError, AuthorizationError } from './errors'; import { Receiver, ReceiverEvent, SayFn, NextMiddleware } from './types'; import { ConversationStore } from './conversation-store'; import { LogLevel } from '@slack/logger'; @@ -288,11 +288,12 @@ describe('App', () => { assert.isAtLeast(fakeLogger.warn.callCount, invalidReceiverEvents.length); }); - it('should warn and skip when a receiver event fails authorization', async () => { + it('should warn, send to global error handler, and skip when a receiver event fails authorization', async () => { // Arrange const fakeLogger = createFakeLogger(); const fakeMiddleware = sinon.fake(noopMiddleware); - const dummyAuthorizationError = new Error(); + const dummyOrigError = new Error('auth failed'); + const dummyAuthorizationError = new AuthorizationError('auth failed', dummyOrigError); const dummyReceiverEvent = createDummyReceiverEvent(); const App = await importApp(); // tslint:disable-line:variable-name @@ -309,6 +310,9 @@ describe('App', () => { // Assert assert(fakeMiddleware.notCalled); assert(fakeLogger.warn.called); + assert.instanceOf(fakeErrorHandler.firstCall.args[0], Error); + assert.propertyVal(fakeErrorHandler.firstCall.args[0], 'code', ErrorCode.AuthorizationError); + assert.propertyVal(fakeErrorHandler.firstCall.args[0], 'original', dummyAuthorizationError.original); }); describe('global middleware', () => {