Skip to content

Commit

Permalink
Add more error handlers to ExpressReceiver (#1406)
Browse files Browse the repository at this point in the history
  • Loading branch information
seratch authored Mar 30, 2022
1 parent 5ed91e0 commit a50879a
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 24 deletions.
88 changes: 64 additions & 24 deletions src/receivers/ExpressReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ import App from '../App';
import {
ReceiverAuthenticityError,
ReceiverInconsistentStateError,
ErrorCode,
CodedError,
} from '../errors';
import { AnyMiddlewareArgs, Receiver, ReceiverEvent } from '../types';
import { verifyRedirectOpts } from './verify-redirect-opts';
import { StringIndexed } from '../types/helpers';
import { HTTPModuleFunctions as httpFunc } from './HTTPModuleFunctions';
import { HTTPModuleFunctions as httpFunc, ReceiverDispatchErrorHandlerArgs, ReceiverProcessEventErrorHandlerArgs, ReceiverUnhandledRequestHandlerArgs } from './HTTPModuleFunctions';
import { HTTPResponseAck } from './HTTPResponseAck';

// Option keys for tls.createServer() and tls.createSecureContext(), exclusive of those for http.createServer()
Expand Down Expand Up @@ -97,6 +96,13 @@ export interface ExpressReceiverOptions {
app?: Application;
router?: IRouter;
customPropertiesExtractor?: (request: Request) => StringIndexed;
dispatchErrorHandler?: (args: ReceiverDispatchErrorHandlerArgs) => Promise<void>;
processEventErrorHandler?: (args: ReceiverProcessEventErrorHandlerArgs) => Promise<boolean>;
// NOTE: for the compatibility with HTTPResponseAck, this handler is not async
// If we receive requests to provide async version of this handler,
// we can add a different name function for it.
unhandledRequestHandler?: (args: ReceiverUnhandledRequestHandlerArgs) => void;
unhandledRequestTimeoutMillis?: number;
}

// Additional Installer Options
Expand Down Expand Up @@ -144,6 +150,14 @@ export default class ExpressReceiver implements Receiver {

private customPropertiesExtractor: (request: Request) => StringIndexed;

private dispatchErrorHandler: (args: ReceiverDispatchErrorHandlerArgs) => Promise<void>;

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

private unhandledRequestHandler: (args: ReceiverUnhandledRequestHandlerArgs) => void;

private unhandledRequestTimeoutMillis: number;

public constructor({
signingSecret = '',
logger = undefined,
Expand All @@ -161,6 +175,10 @@ export default class ExpressReceiver implements Receiver {
app = undefined,
router = undefined,
customPropertiesExtractor = (_req) => ({}),
dispatchErrorHandler = httpFunc.defaultAsyncDispatchErrorHandler,
processEventErrorHandler = httpFunc.defaultProcessEventErrorHandler,
unhandledRequestHandler = httpFunc.defaultUnhandledRequestHandler,
unhandledRequestTimeoutMillis = 3001,
}: ExpressReceiverOptions) {
this.app = app !== undefined ? app : express();

Expand Down Expand Up @@ -190,6 +208,10 @@ export default class ExpressReceiver implements Receiver {
});

this.customPropertiesExtractor = customPropertiesExtractor;
this.dispatchErrorHandler = dispatchErrorHandler;
this.processEventErrorHandler = processEventErrorHandler;
this.unhandledRequestHandler = unhandledRequestHandler;
this.unhandledRequestTimeoutMillis = unhandledRequestTimeoutMillis;

// Verify redirect options if supplied, throws coded error if invalid
verifyRedirectOpts({ redirectUri, redirectUriPath: installerOptions.redirectUriPath });
Expand Down Expand Up @@ -235,22 +257,40 @@ export default class ExpressReceiver implements Receiver {
installerOptions.redirectUriPath;
const { callbackOptions, stateVerification } = installerOptions;
this.router.use(redirectUriPath, async (req, res) => {
if (stateVerification === false) {
// when stateVerification is disabled pass install options directly to handler
// since they won't be encoded in the state param of the generated url
await installer.handleCallback(req, res, callbackOptions, installUrlOptions);
} else {
await installer.handleCallback(req, res, callbackOptions);
try {
if (stateVerification === false) {
// when stateVerification is disabled pass install options directly to handler
// since they won't be encoded in the state param of the generated url
await installer.handleCallback(req, res, callbackOptions, installUrlOptions);
} else {
await installer.handleCallback(req, res, callbackOptions);
}
} catch (e) {
await this.dispatchErrorHandler({
error: e as Error | CodedError,
logger: this.logger,
request: req,
response: res,
});
}
});

const installPath = installerOptions.installPath === undefined ? '/slack/install' : installerOptions.installPath;
const { installPathOptions } = installerOptions;
this.router.get(installPath, async (req, res, next) => {
try {
await installer.handleInstallPath(req, res, installPathOptions, installUrlOptions);
} catch (error) {
next(error);
try {
await installer.handleInstallPath(req, res, installPathOptions, installUrlOptions);
} catch (error) {
next(error);
}
} catch (e) {
await this.dispatchErrorHandler({
error: e as Error | CodedError,
logger: this.logger,
request: req,
response: res,
});
}
});
}
Expand All @@ -262,7 +302,8 @@ export default class ExpressReceiver implements Receiver {
const ack = new HTTPResponseAck({
logger: this.logger,
processBeforeResponse: this.processBeforeResponse,
unhandledRequestTimeoutMillis: 3001, // TODO: this.unhandledRequestTimeoutMillis
unhandledRequestHandler: this.unhandledRequestHandler,
unhandledRequestTimeoutMillis: this.unhandledRequestTimeoutMillis,
httpRequest: req,
httpResponse: res,
});
Expand All @@ -281,19 +322,18 @@ export default class ExpressReceiver implements Receiver {
this.logger.debug('stored response sent');
}
} catch (err) {
const e = err as any;
if ('code' in e) {
// CodedError has code: string
const errorCode = (err as CodedError).code;
if (errorCode === ErrorCode.AuthorizationError) {
// authorize function threw an exception, which means there is no valid installation data
httpFunc.buildNoBodyResponse(res, 401);
ack.ack();
return;
}
const acknowledgedByHandler = await this.processEventErrorHandler({
error: err as Error | CodedError,
logger: this.logger,
request: req,
response: res,
storedResponse: ack.storedResponse,
});
if (acknowledgedByHandler) {
// If the value is false, we don't touch the value as a race condition
// with ack() call may occur especially when processBeforeResponse: false
ack.ack();
}
httpFunc.buildNoBodyResponse(res, 500);
throw err;
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/receivers/HTTPModuleFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ export class HTTPModuleFunctions {
response.end();
}

public static async defaultAsyncDispatchErrorHandler(args: ReceiverDispatchErrorHandlerArgs): Promise<void> {
return HTTPModuleFunctions.defaultDispatchErrorHandler(args);
}

// The default processEventErrorHandler implementation:
// Developers can customize this behavior by passing processEventErrorHandler to the constructor
public static async defaultProcessEventErrorHandler(
Expand Down

0 comments on commit a50879a

Please sign in to comment.