Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Make middleware pipeline async with promises #238

Closed
wants to merge 1 commit into from
Closed

Suggestion: Make middleware pipeline async with promises #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/App.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ describe('App', () => {

// Act
const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
app.use((args) => {
app.use(async (args) => {
// By definition, these events should all produce a say function, so we cast args.say into a SayFn
const say = (args as any).say as SayFn;
say(dummyMessage);
Expand Down Expand Up @@ -425,7 +425,7 @@ describe('App', () => {

// Act
const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
app.use((args) => {
app.use(async (args) => {
// By definition, these events should all produce a say function, so we cast args.say into a SayFn
const say = (args as any).say as SayFn;
say(dummyMessage);
Expand Down Expand Up @@ -506,7 +506,7 @@ describe('App', () => {

// Act
const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
app.use((args) => {
app.use(async (args) => {
assert.isUndefined((args as any).say);
// If the above assertion fails, then it would throw an AssertionError and the following line will not be
// called
Expand All @@ -530,7 +530,7 @@ describe('App', () => {

// Act
const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
app.use((args) => {
app.use(async (args) => {
// By definition, these events should all produce a say function, so we cast args.say into a SayFn
const say = (args as any).say as SayFn;
say(dummyMessage);
Expand Down Expand Up @@ -653,7 +653,7 @@ function createDummyReceiverEvent(): ReceiverEvent {
}

// Utility functions
const noop = () => { }; // tslint:disable-line:no-empty
const noop = async () => { }; // tslint:disable-line:no-empty
const noopMiddleware = ({ next }: { next: NextMiddleware; }) => { next(); };
const noopAuthorize = (() => Promise.resolve({}));

Expand Down
46 changes: 24 additions & 22 deletions src/App.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,10 @@ export default class App {
// Factory for say() utility
const createSay = (channelId: string): SayFn => {
const token = context.botToken !== undefined ? context.botToken : context.userToken;
return (message: Parameters<SayFn>[0]) => {
return async (message: Parameters<SayFn>[0]) => {
const postMessageArguments: ChatPostMessageArguments = (typeof message === 'string') ?
{ token, text: message, channel: channelId } : { ...message, token, channel: channelId };
this.client.chat.postMessage(postMessageArguments)
await this.client.chat.postMessage(postMessageArguments)
.catch(error => this.onGlobalError(error));
};
};
Expand Down Expand Up @@ -410,28 +410,30 @@ export default class App {
}

// Dispatch event through global middleware
processMiddleware(
listenerArgs as AnyMiddlewareArgs,
await processMiddleware(
listenerArgs as AnyMiddlewareArgs,
this.middleware,
(globalProcessedContext: Context, globalProcessedArgs: AnyMiddlewareArgs, startGlobalBubble) => {
this.listeners.forEach((listenerMiddleware) => {
// Dispatch event through all listeners
processMiddleware(
globalProcessedArgs,
listenerMiddleware,
(_listenerProcessedContext, _listenerProcessedArgs, startListenerBubble) => {
startListenerBubble();
},
(error) => {
startGlobalBubble(error);
},
globalProcessedContext,
);
});
async (globalProcessedContext: Context, globalProcessedArgs: AnyMiddlewareArgs, startGlobalBubble) => {
// Dispatch event through all listeners
await Promise.all(
this.listeners.map(async (listenerMiddleware) => {
await processMiddleware(
globalProcessedArgs,
listenerMiddleware,
async (_listenerProcessedContext, _listenerProcessedArgs, startListenerBubble) => {
await startListenerBubble();
},
async (error) => {
await startGlobalBubble(error);
},
globalProcessedContext,
);
})
)
},
(globalError?: CodedError | Error) => {
async (globalError?: CodedError | Error) => {
if (globalError !== undefined) {
this.onGlobalError(globalError);
await this.onGlobalError(globalError);
}
},
context,
Expand All @@ -441,7 +443,7 @@ export default class App {
/**
* Global error handler. The final destination for all errors (hopefully).
*/
private onGlobalError(error: Error): void {
private async onGlobalError(error: Error): Promise<void> {
this.errorHandler(asCodedError(error));
}

Expand Down
2 changes: 1 addition & 1 deletion src/ExpressReceiver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export default class ExpressReceiver extends EventEmitter implements Receiver {
};

if (req.body && req.body.response_url) {
event.respond = (response): void => {
event.respond = async (response): Promise<void> => {
axios.post(req.body.response_url, response)
.catch((e) => {
this.emit('error', e);
Expand Down
6 changes: 3 additions & 3 deletions src/conversation-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ export function conversationContext<ConversationState = any>(
store: ConversationStore<ConversationState>,
logger: Logger,
): Middleware<AnyMiddlewareArgs> {
return (args) => {
return async (args) => {
const { body, context, next } = args;
const { conversationId } = getTypeAndConversation(body as any);
if (conversationId !== undefined) {
// TODO: expiresAt is not passed through to store.set
context.updateConversation = (conversation: ConversationState) => store.set(conversationId, conversation);
store.get(conversationId)
await store.get(conversationId)
.then((conversationState) => {
context.conversation = conversationState;
logger.debug(`Conversation context loaded for ID ${conversationId}`);
Expand All @@ -72,7 +72,7 @@ export function conversationContext<ConversationState = any>(
.then(next);
} else {
logger.debug('No conversation ID for incoming event');
next();
await next();
}
};
}
44 changes: 22 additions & 22 deletions src/middleware/builtin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,53 +21,53 @@ import { ErrorCode, errorWithCode } from '../errors';
/**
* Middleware that filters out any event that isn't an action
*/
export const onlyActions: Middleware<AnyMiddlewareArgs & { action?: SlackAction }> = ({ action, next }) => {
export const onlyActions: Middleware<AnyMiddlewareArgs & { action?: SlackAction }> = async ({ action, next }) => {
// Filter out any non-actions
if (action === undefined) {
return;
}

// It matches so we should continue down this middleware listener chain
next();
await next();
};

/**
* Middleware that filters out any event that isn't a command
*/
export const onlyCommands: Middleware<AnyMiddlewareArgs & { command?: SlashCommand }> = ({ command, next }) => {
export const onlyCommands: Middleware<AnyMiddlewareArgs & { command?: SlashCommand }> = async ({ command, next }) => {
// Filter out any non-commands
if (command === undefined) {
return;
}

// It matches so we should continue down this middleware listener chain
next();
await next();
};

/**
* Middleware that filters out any event that isn't an options
*/
export const onlyOptions: Middleware<AnyMiddlewareArgs & { options?: OptionsRequest }> = ({ options, next }) => {
export const onlyOptions: Middleware<AnyMiddlewareArgs & { options?: OptionsRequest }> = async ({ options, next }) => {
// Filter out any non-options requests
if (options === undefined) {
return;
}

// It matches so we should continue down this middleware listener chain
next();
await next();
};

/**
* Middleware that filters out any event that isn't an event
*/
export const onlyEvents: Middleware<AnyMiddlewareArgs & { event?: SlackEvent }> = ({ event, next }) => {
export const onlyEvents: Middleware<AnyMiddlewareArgs & { event?: SlackEvent }> = async ({ event, next }) => {
// Filter out any non-events
if (event === undefined) {
return;
}

// It matches so we should continue down this middleware listener chain
next();
await next();
};

/**
Expand All @@ -76,7 +76,7 @@ export const onlyEvents: Middleware<AnyMiddlewareArgs & { event?: SlackEvent }>
export function matchConstraints(
constraints: ActionConstraints,
): Middleware<SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs> {
return ({ payload, body, next, context }) => {
return async ({ payload, body, next, context }) => {
// TODO: is putting matches in an array actually helpful? there's no way to know which of the regexps contributed
// which matches (and in which order)
let tempMatches: RegExpMatchArray | null;
Expand Down Expand Up @@ -140,15 +140,15 @@ export function matchConstraints(
}
}

next();
await next();
};
}

/*
* Middleware that filters out messages that don't match pattern
*/
export function matchMessage(pattern: string | RegExp): Middleware<SlackEventMiddlewareArgs<'message'>> {
return ({ message, context, next }) => {
return async ({ message, context, next }) => {
let tempMatches: RegExpMatchArray | null;

if (message.text === undefined) {
Expand All @@ -170,40 +170,40 @@ export function matchMessage(pattern: string | RegExp): Middleware<SlackEventMid
}
}

next();
await next();
};
}

/**
* Middleware that filters out any command that doesn't match name
*/
export function matchCommandName(name: string): Middleware<SlackCommandMiddlewareArgs> {
return ({ command, next }) => {
return async ({ command, next }) => {
// Filter out any commands that are not the correct command name
if (name !== command.command) {
return;
}

next();
await next();
};
}

/**
* Middleware that filters out any event that isn't of given type
*/
export function matchEventType(type: string): Middleware<SlackEventMiddlewareArgs> {
return ({ event, next }) => {
return async ({ event, next }) => {
// Filter out any events that are not the correct type
if (type !== event.type) {
return;
}

next();
await next();
};
}

export function ignoreSelf(): Middleware<AnyMiddlewareArgs> {
return (args) => {
return async (args) => {
// When context does not have a botId in it, then this middleware cannot perform its job. Bail immediately.
if (args.context.botId === undefined) {
args.next(contextMissingPropertyError(
Expand Down Expand Up @@ -237,22 +237,22 @@ export function ignoreSelf(): Middleware<AnyMiddlewareArgs> {
}

// If all the previous checks didn't skip this message, then its okay to resume to next
args.next();
await args.next();
};
}

export function subtype(subtype: string): Middleware<SlackEventMiddlewareArgs<'message'>> {
return ({ message, next }) => {
return async ({ message, next }) => {
if (message.subtype === subtype) {
next();
await next();
}
};
}

const slackLink = /<(?<type>[@#!])?(?<link>[^>|]+)(?:\|(?<label>[^>]+))?>/;

export function directMention(): Middleware<SlackEventMiddlewareArgs<'message'>> {
return ({ message, context, next }) => {
return async ({ message, context, next }) => {
// When context does not have a botUserId in it, then this middleware cannot perform its job. Bail immediately.
if (context.botUserId === undefined) {
next(contextMissingPropertyError(
Expand All @@ -279,7 +279,7 @@ export function directMention(): Middleware<SlackEventMiddlewareArgs<'message'>>
return;
}

next();
await next();
};
}

Expand Down
Loading