Skip to content

Commit

Permalink
Fix #1404 SocketModeReceiver app process exits when any of its event …
Browse files Browse the repository at this point in the history
…listeners throws an exception (#1405)

Co-authored-by: Fil Maj <maj.fil@gmail.com>
  • Loading branch information
seratch and filmaj authored Mar 30, 2022
1 parent 2f97a90 commit d5b3222
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 43 deletions.
85 changes: 44 additions & 41 deletions examples/socket-mode/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@ const clientOptions = {

const app = new App({
// receiver: socketModeReceiver,
token: process.env.BOT_TOKEN, //disable this if enabling OAuth in socketModeReceiver
token: process.env.SLACK_BOT_TOKEN, //disable this if enabling OAuth in socketModeReceiver
// logLevel: LogLevel.DEBUG,
clientOptions,
appToken: process.env.APP_TOKEN,
appToken: process.env.SLACK_APP_TOKEN,
socketMode: true,
logLevel: LogLevel.DEBUG,
});

(async () => {
Expand All @@ -40,9 +41,9 @@ const app = new App({
app.event('app_home_opened', async ({ event, client }) => {
await client.views.publish({
user_id: event.user,
view: {
"type":"home",
"blocks":[
view: {
"type": "home",
"blocks": [
{
"type": "section",
"block_id": "section678",
Expand Down Expand Up @@ -103,8 +104,7 @@ app.shortcut('launch_shortcut', async ({ shortcut, body, ack, context, client })
]
}
});
}
catch (error) {
} catch (error) {
console.error(error);
}
});
Expand All @@ -114,12 +114,44 @@ app.shortcut('launch_shortcut', async ({ shortcut, body, ack, context, client })
// need app_mentions:read and chat:write scopes
app.event('app_mention', async ({ event, context, client, say }) => {
try {
await say({"blocks": [
await say({
"blocks": [
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": `Thanks for the mention <@${event.user}>! Click my fancy button`
},
"accessory": {
"type": "button",
"text": {
"type": "plain_text",
"text": "Button",
"emoji": true
},
"value": "click_me_123",
"action_id": "first_button"
}
}
]
});
} catch (error) {
console.error(error);
}
});

// subscribe to `message.channels` event in your App Config
// need channels:read scope
app.message('hello', async ({ message, say }) => {
// say() sends a message to the channel where the event was triggered
// no need to directly use 'chat.postMessage', no need to include token
await say({
"blocks": [
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": `Thanks for the mention <@${event.user}>! Click my fancy button`
"text": `Thanks for the mention <@${message.user}>! Click my fancy button`
},
"accessory": {
"type": "button",
Expand All @@ -132,41 +164,12 @@ app.event('app_mention', async ({ event, context, client, say }) => {
"action_id": "first_button"
}
}
]});
}
catch (error) {
console.error(error);
}
});

// subscribe to `message.channels` event in your App Config
// need channels:read scope
app.message('hello', async ({ message, say }) => {
// say() sends a message to the channel where the event was triggered
// no need to directly use 'chat.postMessage', no need to include token
await say({"blocks": [
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": `Thanks for the mention <@${message.user}>! Click my fancy button`
},
"accessory": {
"type": "button",
"text": {
"type": "plain_text",
"text": "Button",
"emoji": true
},
"value": "click_me_123",
"action_id": "first_button"
}
}
]});
]
});
});

// Listen and respond to button click
app.action('first_button', async({action, ack, say, context}) => {
app.action('first_button', async ({ action, ack, say, context }) => {
console.log('button clicked');
console.log(action);
// acknowledge the request right away
Expand Down
11 changes: 11 additions & 0 deletions examples/socket-mode/link.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash

current_dir=`dirname $0`
cd ${current_dir}
npm unlink @slack/bolt \
&& npm i \
&& cd ../.. \
&& npm link \
&& cd - \
&& npm i \
&& npm link @slack/bolt
5 changes: 5 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ export {
} from './receivers/HTTPModuleFunctions';
export { HTTPResponseAck } from './receivers/HTTPResponseAck';

export {
SocketModeFunctions,
SocketModeReceiverProcessEventErrorHandlerArgs,
} from './receivers/SocketModeFunctions';

export * from './errors';
export * from './middleware/builtin';
export * from './types';
Expand Down
2 changes: 1 addition & 1 deletion src/receivers/HTTPModuleFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export interface RequestVerificationOptions {
logger?: Logger;
}

// which handles errors occurred while dispatching a rqeuest
// which handles errors occurred while dispatching a request
export interface ReceiverDispatchErrorHandlerArgs {
error: Error | CodedError;
logger: Logger;
Expand Down
42 changes: 42 additions & 0 deletions src/receivers/SocketModeFunctions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import 'mocha';
import { assert } from 'chai';
import { SocketModeFunctions as func } from './SocketModeFunctions';
import {
ReceiverMultipleAckError,
AuthorizationError,
} from '../errors';
import { createFakeLogger } from '../test-helpers';
import { ReceiverEvent } from '../types';

describe('SocketModeFunctions', async () => {
describe('Error handlers for event processing', async () => {
const logger = createFakeLogger();

describe('defaultProcessEventErrorHandler', async () => {
it('should return false if passed any Error other than AuthorizationError', async () => {
const event: ReceiverEvent = {
ack: async () => {},
body: {},
};
const shouldBeAcked = await func.defaultProcessEventErrorHandler({
error: new ReceiverMultipleAckError(),
logger,
event,
});
assert.isFalse(shouldBeAcked);
});
it('should return true if passed an AuthorizationError', async () => {
const event: ReceiverEvent = {
ack: async () => {},
body: {},
};
const shouldBeAcked = await func.defaultProcessEventErrorHandler({
error: new AuthorizationError('msg', new Error()),
logger,
event,
});
assert.isTrue(shouldBeAcked);
});
});
});
});
38 changes: 38 additions & 0 deletions src/receivers/SocketModeFunctions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/* eslint-disable import/prefer-default-export */
import type { Logger } from '@slack/logger';
import { CodedError, ErrorCode } from '../errors';
import { ReceiverEvent } from '../types';

export class SocketModeFunctions {
// ------------------------------------------
// Error handlers for event processing
// ------------------------------------------

// The default processEventErrorHandler implementation:
// Developers can customize this behavior by passing processEventErrorHandler to the constructor
public static async defaultProcessEventErrorHandler(
args: SocketModeReceiverProcessEventErrorHandlerArgs,
): Promise<boolean> {
const { error, logger, event } = args;
// TODO: more details like envelop_id, payload type etc. here
// To make them available, we need to enhance underlying SocketModeClient
// to return more properties to 'slack_event' listeners
logger.error(`An unhandled error occurred while Bolt processed (type: ${event.body.type}, error: ${error})`);
logger.debug(`Error details: ${error}, retry num: ${event.retryNum}, retry reason: ${event.retryReason}`);
const errorCode = (error as CodedError).code;
if (errorCode === ErrorCode.AuthorizationError) {
// The `authorize` function threw an exception, which means there is no valid installation data.
// In this case, we can tell the Slack server-side to stop retries.
return true;
}
return false;
}
}

// The arguments for the processEventErrorHandler,
// which handles errors `await app.processEvent(even)` method throws
export interface SocketModeReceiverProcessEventErrorHandlerArgs {
error: Error | CodedError;
logger: Logger;
event: ReceiverEvent,
}
23 changes: 22 additions & 1 deletion src/receivers/SocketModeReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ import { Logger, ConsoleLogger, LogLevel } from '@slack/logger';
import { InstallProvider, CallbackOptions, InstallProviderOptions, InstallURLOptions, InstallPathOptions } from '@slack/oauth';
import { AppsConnectionsOpenResponse } from '@slack/web-api';
import App from '../App';
import { CodedError } from '../errors';
import { Receiver, ReceiverEvent } from '../types';
import { StringIndexed } from '../types/helpers';
import { buildReceiverRoutes, ReceiverRoutes } from './custom-routes';
import { verifyRedirectOpts } from './verify-redirect-opts';
import {
SocketModeFunctions as socketModeFunc,
SocketModeReceiverProcessEventErrorHandlerArgs,
} from './SocketModeFunctions';

// TODO: we throw away the key names for endpoints, so maybe we should use this interface. is it better for migrations?
// if that's the reason, let's document that with a comment.
Expand All @@ -26,6 +31,7 @@ export interface SocketModeReceiverOptions {
appToken: string; // App Level Token
customRoutes?: CustomRoute[];
customPropertiesExtractor?: (args: any) => StringIndexed;
processEventErrorHandler?: (args: SocketModeReceiverProcessEventErrorHandlerArgs) => Promise<boolean>;
}

export interface CustomRoute {
Expand Down Expand Up @@ -74,6 +80,8 @@ export default class SocketModeReceiver implements Receiver {

private routes: ReceiverRoutes;

private processEventErrorHandler: (args: SocketModeReceiverProcessEventErrorHandlerArgs) => Promise<boolean>;

public constructor({
appToken,
logger = undefined,
Expand All @@ -87,6 +95,7 @@ export default class SocketModeReceiver implements Receiver {
installerOptions = {},
customRoutes = [],
customPropertiesExtractor = (_args) => ({}),
processEventErrorHandler = socketModeFunc.defaultProcessEventErrorHandler,
}: SocketModeReceiverOptions) {
this.client = new SocketModeClient({
appToken,
Expand All @@ -101,6 +110,7 @@ export default class SocketModeReceiver implements Receiver {
return defaultLogger;
})();
this.routes = buildReceiverRoutes(customRoutes);
this.processEventErrorHandler = processEventErrorHandler;

// Verify redirect options if supplied, throws coded error if invalid
verifyRedirectOpts({ redirectUri, redirectUriPath: installerOptions.redirectUriPath });
Expand Down Expand Up @@ -204,7 +214,18 @@ export default class SocketModeReceiver implements Receiver {
retryReason: retry_reason,
customProperties: customPropertiesExtractor(args),
};
await this.app?.processEvent(event);
try {
await this.app?.processEvent(event);
} catch (error) {
const shouldBeAcked = await this.processEventErrorHandler({
error: error as Error | CodedError,
logger: this.logger,
event,
});
if (shouldBeAcked) {
await ack();
}
}
});
}

Expand Down

0 comments on commit d5b3222

Please sign in to comment.