Skip to content

Commit

Permalink
Fix #860 Enable developers to customize the built-in receivers more
Browse files Browse the repository at this point in the history
  • Loading branch information
@seratch
seratch committed Oct 29, 2021
1 parent 15a9205 commit a2da515
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 34 deletions.
23 changes: 22 additions & 1 deletion examples/custom-properties/http.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,28 @@ const app = new App({
"headers": req.headers,
"foo": "bar",
};
}
},
// other custom handlers
dispatchErrorHandler: ({ error, logger, response }) => {
logger.error(`dispatch error: ${error}`);
response.writeHead(404);
response.write("Something is wrong!");
response.end();
},
processEventErrorHandler: ({ error, logger, response }) => {
logger.error(`processEvent error: ${error}`);
// acknowledge it anyway!
response.writeHead(200);
response.end();
return true;
},
unhandledRequestHandler: async ({ logger, response }) => {
// acknowledge it anyway!
logger.info('Acknowledging this incoming request because 2 seconds already passed...');
response.writeHead(200);
response.end();
},
unhandledRequestTimeoutMillis: 2000, // the default is 3001
}),
});

Expand Down
20 changes: 20 additions & 0 deletions src/receivers/HTTPReceiver.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,26 @@ describe('HTTPReceiver', function () {
userScopes: ['chat:write'],
},
customPropertiesExtractor: (req) => ({ headers: req.headers }),
dispatchErrorHandler: ({ error, logger, response }) => {
logger.error(`An unhandled request detected: ${error}`);
response.writeHead(500);
response.write('Something is wrong!');
response.end();
},
processEventErrorHandler: async ({ error, logger, response }) => {
logger.error(`processEvent error: ${error}`);
// acknowledge it anyway!
response.writeHead(200);
response.end();
return true;
},
unhandledRequestHandler: ({ logger, response }) => {
// acknowledge it anyway!
logger.info('Acknowledging this incoming request because 2 seconds already passed...');
response.writeHead(200);
response.end();
},
unhandledRequestTimeoutMillis: 2000, // the default is 3001
});
assert.isNotNull(receiver);
});
Expand Down
165 changes: 132 additions & 33 deletions src/receivers/HTTPReceiver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const httpsOptionKeys = [

const missingServerErrorDescription = 'The receiver cannot be started because private state was mutated. Please report this to the maintainers.';

// All the available arguments in the constructor
export interface HTTPReceiverOptions {
signingSecret: string;
endpoints?: string | string[];
Expand All @@ -75,7 +76,15 @@ export interface HTTPReceiverOptions {
scopes?: InstallURLOptions['scopes'];
installerOptions?: HTTPReceiverInstallerOptions;
customPropertiesExtractor?: (request: BufferedIncomingMessage) => StringIndexed;
// NOTE: As http.RequestListener is not an async function, this cannot be async
dispatchErrorHandler?: (args: HTTPReceiverDispatchErrorHandlerArgs) => void;
processEventErrorHandler?: (args: HTTPReceiverProcessEventErrorHandlerArgs) => Promise<boolean>;
// NOTE: As we use setTimeout under the hood, this cannot be async
unhandledRequestHandler?: (args: HTTPReceiverUnhandledRequestHandlerArgs) => void;
unhandledRequestTimeoutMillis?: number;
}

// All the available argument for OAuth flow enabled apps
export interface HTTPReceiverInstallerOptions {
installPath?: string;
directInstall?: boolean; // see https://api.slack.com/start/distributing/directory#direct_install
Expand All @@ -91,6 +100,85 @@ export interface HTTPReceiverInstallerOptions {
callbackOptions?: CallbackOptions;
}

// The arguments for the dispatchErrorHandler,
// which handles errors occurred while dispatching a rqeuest
export interface HTTPReceiverDispatchErrorHandlerArgs {
error: Error | CodedError;
logger: Logger;
request: IncomingMessage;
response: ServerResponse;
}

// The default dispathErrorHandler implementation:
// Developers can customize this behavior by passing dispatchErrorHandler to the constructor
// Note that it was not possible to make this function async due to the limitation of http module
function defaultDispatchErrorHandler(args: HTTPReceiverDispatchErrorHandlerArgs) {
const { error, logger, request, response } = args;
if ('code' in error) {
if (error.code === ErrorCode.HTTPReceiverDeferredRequestError) {
logger.info(`Unhandled HTTP request (${request.method}) made to ${request.url}`);
response.writeHead(404);
response.end();
return;
}
}
logger.error(`An unexpected error occurred during a request (${request.method}) made to ${request.url}`);
logger.debug(`Error details: ${error}`);
response.writeHead(500);
response.end();
}

// The arguments for the processEventErrorHandler,
// which handles errors `await app.processEvent(even)` method throws
export interface HTTPReceiverProcessEventErrorHandlerArgs {
error: Error | CodedError;
logger: Logger;
request: IncomingMessage;
response: ServerResponse;
storedResponse: any;
}

// The default processEventErrorHandler implementation:
// Developers can customize this behavior by passing processEventErrorHandler to the constructor
async function defaultProcessEventErrorHandler(args: HTTPReceiverProcessEventErrorHandlerArgs): Promise<boolean> {
const { error, response, logger, storedResponse } = args;
if ('code' in error) {
// CodedError has code: string
const errorCode = (error as CodedError).code;
if (errorCode === ErrorCode.AuthorizationError) {
// authorize function threw an exception, which means there is no valid installation data
response.writeHead(401);
response.end();
return true;
}
}
logger.error('An unhandled error occurred while Bolt processed an event');
logger.debug(`Error details: ${error}, storedResponse: ${storedResponse}`);
response.writeHead(500);
response.end();
return false;
}

// The arguments for the unhandledRequestHandler,
// which deals with any unhandled incoming requests from Slack.
// (The default behavior is just printing error logs)
export interface HTTPReceiverUnhandledRequestHandlerArgs {
logger: Logger;
request: IncomingMessage;
response: ServerResponse;
}

// The default unhandledRequestHandler implementation:
// Developers can customize this behavior by passing unhandledRequestHandler to the constructor
// Note that this method cannot be an async function to align with the implementation using setTimeout
function defaultUnhandledRequestHandler(args: HTTPReceiverUnhandledRequestHandlerArgs): void {
const { logger } = args;
logger.error(
'An incoming event was not acknowledged within 3 seconds. ' +
'Ensure that the ack() argument is called in a listener.',
);
}

/**
* Receives HTTP requests with Events, Slash Commands, and Actions
*/
Expand Down Expand Up @@ -131,6 +219,14 @@ export default class HTTPReceiver implements Receiver {

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

private dispatchErrorHandler: (args: HTTPReceiverDispatchErrorHandlerArgs) => void;

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

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

private unhandledRequestTimeoutMillis: number;

public constructor({
signingSecret = '',
endpoints = ['/slack/events'],
Expand All @@ -147,6 +243,10 @@ export default class HTTPReceiver implements Receiver {
scopes = undefined,
installerOptions = {},
customPropertiesExtractor = (_req) => ({}),
dispatchErrorHandler = defaultDispatchErrorHandler,
processEventErrorHandler = defaultProcessEventErrorHandler,
unhandledRequestHandler = defaultUnhandledRequestHandler,
unhandledRequestTimeoutMillis = 3001,
}: HTTPReceiverOptions) {
// Initialize instance variables, substituting defaults for each value
this.signingSecret = signingSecret;
Expand Down Expand Up @@ -203,6 +303,10 @@ export default class HTTPReceiver implements Receiver {
installerOptions.renderHtmlForInstallPath :
defaultRenderHtmlForInstallPath;
this.customPropertiesExtractor = customPropertiesExtractor;
this.dispatchErrorHandler = dispatchErrorHandler;
this.processEventErrorHandler = processEventErrorHandler;
this.unhandledRequestHandler = unhandledRequestHandler;
this.unhandledRequestTimeoutMillis = unhandledRequestTimeoutMillis;

// Assign the requestListener property by binding the unboundRequestListener to this instance
this.requestListener = this.unboundRequestListener.bind(this);
Expand Down Expand Up @@ -240,17 +344,14 @@ export default class HTTPReceiver implements Receiver {
try {
this.requestListener(req, res);
} catch (error) {
const e = error as any;
if (e.code === ErrorCode.HTTPReceiverDeferredRequestError) {
this.logger.info(`Unhandled HTTP request (${req.method}) made to ${req.url}`);
res.writeHead(404);
res.end();
} else {
this.logger.error(`An unexpected error occurred during a request (${req.method}) made to ${req.url}`);
this.logger.debug(`Error details: ${e}`);
res.writeHead(500);
res.end();
}
// You may get an error here only when the requestListener failed
// to start processing incoming requests, or your app receives a request to an unexpected path.
this.dispatchErrorHandler({
error: error as Error | CodedError,
logger: this.logger,
request: req,
response: res,
});
}
});

Expand Down Expand Up @@ -349,6 +450,9 @@ export default class HTTPReceiver implements Receiver {

// If the request did not match the previous conditions, an error is thrown. The error can be caught by the
// the caller in order to defer to other routing logic (similar to calling `next()` in connect middleware).
// If you would like to customize the HTTP repsonse for this pattern,
// implement your own dispatchErrorHandler that handles an exception
// with ErrorCode.HTTPReceiverDeferredRequestError.
throw new HTTPReceiverDeferredRequestError(`Unhandled HTTP request (${method}) made to ${path}`, req, res);
}

Expand Down Expand Up @@ -408,12 +512,13 @@ export default class HTTPReceiver implements Receiver {
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.',
);
this.unhandledRequestHandler({
logger: this.logger,
request: req,
response: res,
});
}
}, 3001);
}, this.unhandledRequestTimeoutMillis);

// Structure the ReceiverEvent
let storedResponse;
Expand All @@ -426,6 +531,8 @@ export default class HTTPReceiver implements Receiver {
}
isAcknowledged = true;
if (this.processBeforeResponse) {
// In the case where processBeforeResponse: true is enabled, we don't send the HTTP response immediately.
// We hold off until the listener execution is completed.
if (!response) {
storedResponse = '';
} else {
Expand Down Expand Up @@ -455,6 +562,7 @@ export default class HTTPReceiver implements Receiver {
try {
await this.app?.processEvent(event);
if (storedResponse !== undefined) {
// in the case of processBeforeResponse: true
if (typeof storedResponse === 'string') {
res.writeHead(200);
res.end(storedResponse);
Expand All @@ -464,23 +572,14 @@ export default class HTTPReceiver 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 = (e as CodedError).code;
if (errorCode === ErrorCode.AuthorizationError) {
// authorize function threw an exception, which means there is no valid installation data
res.writeHead(401);
res.end();
isAcknowledged = true;
return;
}
}
this.logger.error('An unhandled error occurred while Bolt processed an event');
this.logger.debug(`Error details: ${e}, storedResponse: ${storedResponse}`);
res.writeHead(500);
res.end();
} catch (error) {
isAcknowledged = await this.processEventErrorHandler({
error: error as Error | CodedError,
logger: this.logger,
request: req,
response: res,
storedResponse,
});
}
})();
}
Expand Down

0 comments on commit a2da515

Please sign in to comment.