adds error codes, with meaningful usage and interfaces

src/App.ts

@@ -34,6 +34,7 @@ import {
   ReceiverEvent,
 } from './types';
 import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers';
+import { ErrorCode, CodedError, errorWithCode, asCodedError } from './errors';
 const packageJson = require('../package.json'); // tslint:disable-line:no-require-imports no-var-requires
 
 /** App initialization options */
@@ -86,7 +87,7 @@ export interface ActionConstraints {
 }
 
 export interface ErrorHandler {
-  (error: Error): void;
+  (error: CodedError): void;
 }
 
 /**
@@ -137,11 +138,17 @@ export default class App {
 
     if (token !== undefined) {
       if (authorize !== undefined) {
-        throw new Error(`Both token and authorize options provided. ${tokenUsage}`);
+        throw errorWithCode(
+          `Both token and authorize options provided. ${tokenUsage}`,
+          ErrorCode.AppInitializationError,
+        );
       }
       this.authorize = singleTeamAuthorization(this.client, { botId, botUserId, botToken: token });
     } else if (authorize === undefined) {
-      throw new Error(`No token and no authorize options provided. ${tokenUsage}`);
+      throw errorWithCode(
+        `No token and no authorize options provided. ${tokenUsage}`,
+        ErrorCode.AppInitializationError,
+      );
     } else {
       this.authorize = authorize;
     }
@@ -154,8 +161,11 @@ export default class App {
       this.receiver = new ExpressReceiver({ signingSecret, endpoints });
     } else if (receiver === undefined) {
       // Check for custom receiver
-      throw new Error('Signing secret not found, so could not initialize the default receiver. Set a signing secret ' +
-        'or use a custom receiver.');
+      throw errorWithCode(
+        'Signing secret not found, so could not initialize the default receiver. Set a signing secret or use a ' +
+        'custom receiver.',
+        ErrorCode.AppInitializationError,
+      );
     } else {
       this.receiver = receiver;
     }
@@ -290,6 +300,9 @@ export default class App {
    * Handles events from the receiver
    */
   private async onIncomingEvent({ body, ack, respond }: ReceiverEvent): Promise<void> {
+    // 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
+    // with "finally" type error situations.
 
     // Introspect the body to determine what type of incoming event is being handled, and any channel context
     const { type, conversationId } = getTypeAndConversation(body);
@@ -304,16 +317,23 @@ export default class App {
     const bodyArg = body as AnyMiddlewareArgs['body'];
 
     // Initialize context (shallow copy to enforce object identity separation)
-    const context: Context = { ...(await this.authorize(buildSource(type, conversationId, bodyArg), bodyArg)) };
+    const source = buildSource(type, conversationId, bodyArg);
+    const authorizeResult = await (this.authorize(source, bodyArg).catch((error) => {
+      this.onGlobalError(authorizationErrorFromOriginal(error));
+    }));
+    if (authorizeResult === undefined) {
+      this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.');
+      return;
+    }
+    const context: Context = { ...authorizeResult };
 
-    // Factory for say() argument
+    // Factory for say() utility
     const createSay = (channelId: string): SayFn => {
       const token = context.botToken !== undefined ? context.botToken : context.userToken;
       return (message: Parameters<SayFn>[0]) => {
         const postMessageArguments: ChatPostMessageArguments = (typeof message === 'string') ?
           { token, text: message, channel: channelId } : { ...message, token, channel: channelId };
         this.client.chat.postMessage(postMessageArguments)
-          // TODO: create a specific error code
           .catch(this.onGlobalError);
       };
     };
@@ -400,7 +420,7 @@ export default class App {
           );
         });
       },
-      (globalError?: Error) => {
+      (globalError?: CodedError | Error) => {
         if (globalError !== undefined) {
           this.onGlobalError(globalError);
         }
@@ -413,7 +433,7 @@ export default class App {
    * Global error handler. The final destination for all errors (hopefully).
    */
   private onGlobalError(error: Error): void {
-    this.errorHandler(error);
+    this.errorHandler(asCodedError(error));
   }
 
 }
@@ -493,3 +513,15 @@ 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;
+}

src/ExpressReceiver.ts

@@ -1,13 +1,16 @@
 import { EventEmitter } from 'events';
-import { Receiver, ReceiverEvent } from './types';
+import { Receiver, ReceiverEvent /*, ReceiverAckTimeoutError */ } from './types';
 import { createServer, Server } from 'http';
-import express, { Request, Response, Application, RequestHandler } from 'express';
+import express, { Request, Response, Application, RequestHandler, NextFunction } from 'express';
 import axios from 'axios';
 import rawBody from 'raw-body';
 import crypto from 'crypto';
 import tsscmp from 'tsscmp';
 import querystring from 'querystring';
+import { ErrorCode, errorWithCode } from './errors';
 
+// 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.
 export interface ExpressReceiverOptions {
   signingSecret: string;
   endpoints?: string | {
@@ -32,6 +35,7 @@ export default class ExpressReceiver extends EventEmitter implements Receiver {
     super();
 
     this.app = express();
+    this.app.use(this.errorHandler.bind(this));
     // TODO: what about starting an https server instead of http? what about other options to create the server?
     this.server = createServer(this.app);
 
@@ -107,6 +111,12 @@ export default class ExpressReceiver extends EventEmitter implements Receiver {
       });
     });
   }
+
+  private errorHandler(err: any, _req: Request, _res: Response, next: NextFunction): void {
+    this.emit('error', err);
+    // Forward to express' default error handler (which knows how to print stack traces in development)
+    next(err);
+  }
 }
 
 // TODO: respond to url_verification, and also help a beginner set up Events API (maybe adopt the CLI verify tool)
@@ -132,8 +142,10 @@ function verifySlackRequest(signingSecret: string): RequestHandler {
       const fiveMinutesAgo = Math.floor(Date.now() / 1000) - (60 * 5);
 
       if (ts < fiveMinutesAgo) {
-        // TODO: coded error
-        const error = new Error('Slack request signing verification failed');
+        const error = errorWithCode(
+          'Slack request signing verification failed. Timestamp is too old.',
+          ErrorCode.ExpressReceiverAuthenticityError,
+        );
         next(error);
       }
 
@@ -142,16 +154,17 @@ function verifySlackRequest(signingSecret: string): RequestHandler {
       hmac.update(`${version}:${ts}:${body}`);
 
       if (!tsscmp(hash, hmac.digest('hex'))) {
-        // TODO: coded error
-        const error = new Error('Slack request signing verification failed');
+        const error = errorWithCode(
+          'Slack request signing verification failed. Signature mismatch.',
+          ErrorCode.ExpressReceiverAuthenticityError,
+        );
         next(error);
       }
 
       // Verification passed, assign string body back to request and resume
       req.body = body;
       next();
     } catch (error) {
-      // TODO: coded error
       next(error);
     }
   };

src/conversation-store.ts

@@ -32,12 +32,10 @@ export class MemoryStore<ConversationState = any> implements ConversationStore<C
         if (entry.expiresAt !== undefined && Date.now() > entry.expiresAt) {
           // release the memory
           this.state.delete(conversationId);
-          // TODO: coded error?
           reject(new Error('Conversation expired'));
         }
         resolve(entry.value);
       }
-      // TODO: coded error?
       reject(new Error('Conversation not found'));
     });
   }

src/errors.ts

@@ -0,0 +1,34 @@
+export interface CodedError extends Error {
+  code: string; // This can be a value from ErrorCode, or WebClient's ErrorCode, or a NodeJS error code
+}
+
+export enum ErrorCode {
+  AppInitializationError = 'slack_bolt_app_initialization_error',
+  AuthorizationError = 'slack_bolt_authorization_error',
+
+  ContextMissingPropertyError = 'slack_bolt_context_missing_property_error',
+
+  ReceiverAckTimeoutError = 'slack_bolt_receiver_ack_timeout_error',
+
+  ExpressReceiverAuthenticityError = 'slack_bolt_express_receiver_authenticity_error',
+
+  /**
+   * This value is used to assign to errors that occur inside the framework but do not have a code, to keep interfaces
+   * in terms of CodedError.
+   */
+  UnknownError = 'slack_bolt_unknown_error',
+}
+
+export function errorWithCode(message: string, code: ErrorCode): CodedError {
+  const error = new Error(message);
+  (error as CodedError).code = code;
+  return error as CodedError;
+}
+
+export function asCodedError(error: CodedError | Error): CodedError {
+  if ((error as CodedError).code !== undefined) {
+    return error as CodedError;
+  }
+  (error as CodedError).code = ErrorCode.UnknownError;
+  return error as CodedError;
+}

src/index.ts

@@ -4,10 +4,13 @@ export {
   Authorize,
   AuthorizeSourceData,
   AuthorizeResult,
+  AuthorizationError,
   ActionConstraints,
   LogLevel,
 } from './App';
 
+export { ErrorCode } from './errors';
+
 export {
   default as ExpressReceiver,
   ExpressReceiverOptions,

src/middleware/builtin.ts

@@ -13,8 +13,10 @@ import {
   DialogSubmitAction,
   MessageAction,
   BlockElementAction,
+  ContextMissingPropertyError,
 } from '../types';
 import { ActionConstraints } from '../App';
+import { ErrorCode, errorWithCode } from '../errors';
 
 /**
  * Middleware that filters out any event that isn't an action
@@ -196,9 +198,10 @@ export function ignoreSelf(): Middleware<AnyMiddlewareArgs> {
   return (args) => {
     // When context does not have a botId in it, then this middleware cannot perform its job. Bail immediately.
     if (args.context.botId === undefined) {
-      // TODO: coded error
-      args.next(new Error('Cannot ignore events from the app without a bot ID. ' +
-        'Ensure authorize callback returns a botId.'));
+      args.next(contextMissingPropertyError(
+        'botId',
+        'Cannot ignore events from the app without a bot ID. Ensure authorize callback returns a botId.',
+      ));
       return;
     }
 
@@ -244,9 +247,10 @@ export function directMention(): Middleware<SlackEventMiddlewareArgs<'message'>>
   return ({ 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) {
-      // TODO: coded error
-      next(new Error('Cannot match direct mentions of the app without a bot user ID. ' +
-        'Ensure authorize callback returns a botUserId.'));
+      next(contextMissingPropertyError(
+        'botUserId',
+        'Cannot match direct mentions of the app without a bot user ID. Ensure authorize callback returns a botUserId.',
+      ));
       return;
     }
 
@@ -290,3 +294,10 @@ function isEventArgs(
 ): args is SlackEventMiddlewareArgs {
   return (args as SlackEventMiddlewareArgs).event !== undefined;
 }
+
+export function contextMissingPropertyError(propertyName: string, message?: string): ContextMissingPropertyError {
+  const m = message === undefined ? `Context missing property: ${propertyName}` : message;
+  const error = errorWithCode(m, ErrorCode.ContextMissingPropertyError);
+  (error as ContextMissingPropertyError).missingProperty = propertyName;
+  return error as ContextMissingPropertyError;
+}

src/types/middleware.ts

@@ -3,6 +3,7 @@ import { SlackEventMiddlewareArgs } from './events';
 import { SlackActionMiddlewareArgs } from './actions';
 import { SlackCommandMiddlewareArgs } from './command';
 import { SlackOptionsMiddlewareArgs } from './options';
+import { CodedError, ErrorCode } from '../errors';
 
 export type AnyMiddlewareArgs =
   SlackEventMiddlewareArgs | SlackActionMiddlewareArgs | SlackCommandMiddlewareArgs | SlackOptionsMiddlewareArgs;
@@ -27,3 +28,8 @@ export interface NextMiddleware {
   (postProcess: PostProcessFn): void;
   (): void;
 }
+
+export interface ContextMissingPropertyError extends CodedError {
+  code: ErrorCode.ContextMissingPropertyError;
+  missingProperty: string;
+}

src/types/receiver.ts

@@ -1,5 +1,6 @@
 import { RespondFn, AckFn } from '../types';
 import { StringIndexed } from './helpers';
+import { CodedError, ErrorCode } from '../errors';
 
 export interface ReceiverEvent {
   body: StringIndexed;
@@ -9,9 +10,14 @@ export interface ReceiverEvent {
   respond?: RespondFn;
 }
 
+// TODO: should it really be a requirement that on() returns this?
 export interface Receiver {
   on(event: 'message', listener: (event: ReceiverEvent) => void): this;
-  on(event: 'error', listener: (error: Error) => void): this;
+  on(event: 'error', listener: (error: Error | ReceiverAckTimeoutError) => void): this;
   start(...args: any[]): Promise<unknown>;
   stop(...args: any[]): Promise<unknown>;
 }
+
+export interface ReceiverAckTimeoutError extends CodedError {
+  code: ErrorCode.ReceiverAckTimeoutError;
+}