Skip to content

Commit

Permalink
Convert receivers to App#processEvent
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Barlock committed Jan 24, 2020
1 parent ccb1c5d commit 269f462
Show file tree
Hide file tree
Showing 17 changed files with 588 additions and 353 deletions.
338 changes: 235 additions & 103 deletions src/App.spec.ts
View file

Large diffs are not rendered by default.

70 changes: 30 additions & 40 deletions src/App.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,15 @@ import {
BlockAction,
InteractiveMessage,
SlackViewAction,
Receiver,
ReceiverEvent,
RespondArguments,
} from './types';
import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers';
import { ErrorCode, CodedError, errorWithCode, asCodedError } from './errors';
import { Receiver, ReceiverEvent } from './receiver';
import {
CodedError,
asCodedError,
AppInitializationError,
} from './errors';
const packageJson = require('../package.json'); // tslint:disable-line:no-require-imports no-var-requires

/** App initialization options */
Expand All @@ -72,7 +75,7 @@ export { LogLevel, Logger } from '@slack/logger';
export interface Authorize {
(
source: AuthorizeSourceData,
body: ReceiverEvent['body'],
body: AnyMiddlewareArgs['body'],
): Promise<AuthorizeResult>;
}

Expand Down Expand Up @@ -107,7 +110,7 @@ export interface ViewConstraints {
}

export interface ErrorHandler {
(error: CodedError): void;
(error: CodedError): Promise<void>;
}

/**
Expand Down Expand Up @@ -176,16 +179,14 @@ export default class App {

if (token !== undefined) {
if (authorize !== undefined) {
throw errorWithCode(
throw new AppInitializationError(
`Both token and authorize options provided. ${tokenUsage}`,
ErrorCode.AppInitializationError,
);
}
this.authorize = singleTeamAuthorization(this.client, { botId, botUserId, botToken: token });
} else if (authorize === undefined) {
throw errorWithCode(
throw new AppInitializationError(
`No token and no authorize options provided. ${tokenUsage}`,
ErrorCode.AppInitializationError,
);
} else {
this.authorize = authorize;
Expand All @@ -200,20 +201,15 @@ export default class App {
} else {
// No custom receiver
if (signingSecret === undefined) {
throw errorWithCode(
'Signing secret not found, so could not initialize the default receiver. Set a signing secret or use a ' +
'custom receiver.',
ErrorCode.AppInitializationError,
throw new AppInitializationError(
'Signing secret not found, so could not initialize the default receiver. Set a signing secret or use a ' +
'custom receiver.',
);
} else {
// Create default ExpressReceiver
this.receiver = new ExpressReceiver({ signingSecret, logger, endpoints, agent, clientTls });
}
// Create default ExpressReceiver
this.receiver = new ExpressReceiver({ signingSecret, logger, endpoints, agent, clientTls });
}

// Subscribe to messages and errors from the receiver
this.receiver.on('message', message => this.onIncomingEvent(message));
this.receiver.on('error', error => this.onGlobalError(error));
this.receiver.init(this);

// Conditionally use a global middleware that ignores events (including messages) that are sent from this app
if (ignoreSelf) {
Expand Down Expand Up @@ -389,7 +385,7 @@ export default class App {
/**
* Handles events from the receiver
*/
private async onIncomingEvent(event: ReceiverEvent): Promise<void> {
public async processEvent(event: ReceiverEvent): Promise<void> {
const { body, ack } = event;
// TODO: when generating errors (such as in the say utility) it may become useful to capture the current context,
// or even all of the args, as properties of the error. This would give error handling code some ability to deal
Expand All @@ -407,13 +403,15 @@ export default class App {

// Initialize context (shallow copy to enforce object identity separation)
const source = buildSource(type, conversationId, bodyArg);
const authorizeResult = await (this.authorize(source, bodyArg).catch((error) => {
this.onGlobalError(authorizationErrorFromOriginal(error));
}));
if (authorizeResult === undefined) {
let authorizeResult;

try {
authorizeResult = await this.authorize(source, bodyArg);
} catch (error) {
this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.');
return;
}

const context: Context = { ...authorizeResult };

// Factory for say() utility
Expand Down Expand Up @@ -496,6 +494,7 @@ export default class App {
listenerArgs.ack = ack;
} else {
// Events API requests are acknowledged right away, since there's no data expected

await ack();
}
const middlewareChain = [...this.middleware];
Expand All @@ -518,15 +517,16 @@ export default class App {
...(listenerArgs as AnyMiddlewareArgs),
});
} catch (error) {
this.onGlobalError(error);
await event.ack(asCodedError(error));
return this.handleError(error);
}
}

/**
* Global error handler. The final destination for all errors (hopefully).
*/
private onGlobalError(error: Error): void {
this.errorHandler(asCodedError(error));
public handleError(error: Error): Promise<void> {
return this.errorHandler(asCodedError(error));
}

}
Expand Down Expand Up @@ -583,6 +583,8 @@ function isBlockActionOrInteractiveMessageBody(
function defaultErrorHandler(logger: Logger): ErrorHandler {
return (error) => {
logger.error(error);

throw error;
};
}

Expand All @@ -609,15 +611,3 @@ function singleTeamAuthorization(

/* Instrumentation */
addAppMetadata({ name: packageJson.name, version: packageJson.version });

/* Error handling helpers */
function authorizationErrorFromOriginal(original: Error): AuthorizationError {
const error = errorWithCode('Authorization of incoming event did not succeed.', ErrorCode.AuthorizationError);
(error as AuthorizationError).original = original;
return error as AuthorizationError;
}

export interface AuthorizationError extends CodedError {
code: ErrorCode.AuthorizationError;
original: Error;
}
31 changes: 11 additions & 20 deletions src/ExpressReceiver.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// tslint:disable:no-implicit-dependencies
import 'mocha';

import { Logger, LogLevel } from '@slack/logger';
import { assert } from 'chai';
import { Request, Response } from 'express';
Expand All @@ -11,11 +10,10 @@ import { Readable } from 'stream';
import ExpressReceiver, {
respondToSslCheck,
respondToUrlVerification,
verifySignatureAndParseBody,
verifySignatureAndParseRawBody,
} from './ExpressReceiver';

describe('ExpressReceiver', () => {

const noopLogger: Logger = {
debug(..._msg: any[]): void { /* noop */ },
info(..._msg: any[]): void { /* noop */ },
Expand Down Expand Up @@ -58,6 +56,7 @@ describe('ExpressReceiver', () => {
signingSecret: 'my-secret',
logger: noopLogger,
});

await receiver.start(9999);
await receiver.stop();
});
Expand Down Expand Up @@ -102,7 +101,7 @@ describe('ExpressReceiver', () => {
});
});

describe('url_verification requset handler', () => {
describe('url_verification request handler', () => {
it('should handle valid requests', async () => {
// Arrange
// tslint:disable-next-line: no-object-literal-type-assertion
Expand Down Expand Up @@ -141,7 +140,7 @@ describe('ExpressReceiver', () => {
});
});

describe('verifySignatureAndParseBody', () => {
describe('verifySignatureAndParseRawBody', () => {

let clock: SinonFakeTimers;

Expand Down Expand Up @@ -196,7 +195,7 @@ describe('ExpressReceiver', () => {
const next = (error: any) => { state.error = error; };

// Act
const verifier = verifySignatureAndParseBody(noopLogger, signingSecret);
const verifier = verifySignatureAndParseRawBody(noopLogger, signingSecret);
await verifier(req, resp, next);
}

Expand Down Expand Up @@ -245,24 +244,15 @@ describe('ExpressReceiver', () => {
const result: any = {};
const resp = buildResponseToVerify(result);

let error: string = '';
let warn: string = '';
const logger = {
error: (msg: string) => { error = msg; },
warn: (msg: string) => { warn = msg; },
} as any as Logger;

const next = sinon.fake();

// Act
const verifier = verifySignatureAndParseBody(logger, signingSecret);
const verifier = verifySignatureAndParseRawBody(noopLogger, signingSecret);
await verifier(req, resp, next);

// Assert
assert.equal(result.code, 400);
assert.equal(result.sent, true);
assert.equal(error, 'Failed to parse body as JSON data for content-type: undefined');
assert.equal(warn, 'Parsing request body failed (error: SyntaxError: Unexpected token o in JSON at position 1)');
}

it('should fail to parse request body without content-type header', async () => {
Expand Down Expand Up @@ -301,7 +291,7 @@ describe('ExpressReceiver', () => {
const next = sinon.fake();

// Act
const verifier = verifySignatureAndParseBody(noopLogger, signingSecret);
const verifier = verifySignatureAndParseRawBody(noopLogger, signingSecret);
await verifier(req, resp, next);

// Assert
Expand Down Expand Up @@ -355,7 +345,8 @@ describe('ExpressReceiver', () => {
const next = sinon.fake();

// Act
const verifier = verifySignatureAndParseBody(noopLogger, signingSecret);

const verifier = verifySignatureAndParseRawBody(noopLogger, signingSecret);
await verifier(req, resp, next);

// Assert
Expand Down Expand Up @@ -388,7 +379,7 @@ describe('ExpressReceiver', () => {
const next = sinon.fake();

// Act
const verifier = verifySignatureAndParseBody(noopLogger, signingSecret);
const verifier = verifySignatureAndParseRawBody(noopLogger, signingSecret);
await verifier(req, resp, next);

// Assert
Expand All @@ -414,7 +405,7 @@ describe('ExpressReceiver', () => {
const next = sinon.fake();

// Act
const verifier = verifySignatureAndParseBody(noopLogger, signingSecret);
const verifier = verifySignatureAndParseRawBody(noopLogger, signingSecret);
verifier(req, resp, next);
await verifier(req, resp, next);

Expand Down
Loading

0 comments on commit 269f462

Please sign in to comment.