Iterate on changes

- Use `RequestHandler` in events-api
- Retype `respond` error to `ResponseError`
- Re-add lodash.isPlainObject to fix failing tests
- Rearrange `action` and `options` to catch errors earlier
- Remove unnecessary cast
- Add extra time to test timeout

packages/events-api/package.json

@@ -44,6 +44,7 @@
   },
   "dependencies": {
     "@types/debug": "^4.1.4",
+    "@types/express": "^4.17.0",
     "@types/lodash.isstring": "^4.0.6",
     "@types/node": ">=4.2.0",
     "@types/yargs": "^13.0.0",

packages/events-api/src/adapter.ts

@@ -1,10 +1,11 @@
 /* tslint:disable import-name */
 import EventEmitter from 'events';
-import http, { IncomingMessage, ServerResponse, RequestListener } from 'http';
+import http, { RequestListener } from 'http';
 import debugFactory from 'debug';
 import isString from 'lodash.isstring';
 import { createHTTPHandler } from './http-handler';
 import { isFalsy } from './util';
+import { RequestHandler } from 'express'; // tslint:disable-line: no-implicit-dependencies
 /* tslint:enable import-name */
 
 const debug = debugFactory('@slack/events-api:adapter');
@@ -118,7 +119,7 @@ export class SlackEventAdapter extends EventEmitter {
   /**
    * Returns a middleware-compatible adapter.
    */
-  public expressMiddleware(): (req: IncomingMessage, res: ServerResponse, next: () => void) => void {
+  public expressMiddleware(): RequestHandler {
     const requestListener = this.requestListener();
     return (req, res, _next) => {
       requestListener(req, res);

packages/events-api/src/http-handler.spec.js

@@ -1,4 +1,3 @@
-require('mocha');
 const { assert } = require('chai');
 const sinon = require('sinon');
 const proxyquire = require('proxyquire');

packages/events-api/src/http-handler.ts

@@ -61,11 +61,11 @@ export function createHTTPHandler(adapter: SlackEventAdapter): HTTPHandler {
       debug('sending response - error: %s, responseOptions: %o', err, responseOptions);
       // Deal with errors up front
       if (!isFalsy(err)) {
-        if ('status' in err) {
+        if ('status' in err && typeof err.status === 'number') {
           res.statusCode = err.status;
         } else if ('code' in err && (
-          err.code === ErrorCode.SignatureVerificationFailure ||
-          err.code === ErrorCode.RequestTimeFailure
+          (err as CodedError).code === ErrorCode.SignatureVerificationFailure ||
+          (err as CodedError).code === ErrorCode.RequestTimeFailure
         )) {
           res.statusCode = ResponseStatus.NotFound;
         } else {
@@ -115,7 +115,8 @@ export function createHTTPHandler(adapter: SlackEventAdapter): HTTPHandler {
         adapter.emit('error', error, respond);
       } else if (process.env.NODE_ENV === 'development') {
         adapter.emit('error', error);
-        respond({ status: ResponseStatus.Failure }, { content: error.message });
+        // tslint:disable-next-line: no-object-literal-type-assertion
+        respond({ status: ResponseStatus.Failure } as ResponseError, { content: error.message });
       } else {
         adapter.emit('error', error);
         respond(error);
@@ -217,14 +218,21 @@ enum ResponseStatus {
 type HTTPHandler = (req: IncomingMessage & { body?: any, rawBody?: Buffer }, res: ServerResponse) => void;
 
 /**
- * A response handler returned by `sendResponse`.
+ * A Node-style response handler that takes an error (if any occurred) and a few response-related options.
  */
-export type ResponseHandler = (err?: Error | CodedError | { status: ResponseStatus }, responseOptions?: {
+export type ResponseHandler = (err?: ResponseError, responseOptions?: {
   failWithNoRetry?: boolean;
   redirectLocation?: boolean;
   content?: any;
 }) => void;
 
+/**
+ * An error (that may or may not have a status code) in response to a request.
+ */
+export interface ResponseError extends Error {
+  status?: number;
+}
+
 /**
  * Parameters for calling {@link verifyRequestSignature}.
  */

packages/interactive-messages/package.json

@@ -56,12 +56,14 @@
     "axios": "^0.18.0",
     "debug": "^3.1.0",
     "lodash.isfunction": "^3.0.9",
+    "lodash.isplainobject": "^4.0.6",
     "lodash.isregexp": "^4.0.1",
     "lodash.isstring": "^4.0.1",
     "raw-body": "^2.3.3",
     "tsscmp": "^1.0.6"
   },
   "devDependencies": {
+    "@types/lodash.isplainobject": "^4.0.6",
     "body-parser": "^1.18.2",
     "chai": "^4.2.0",
     "codecov": "^3.0.0",

packages/interactive-messages/src/adapter.spec.js

@@ -397,7 +397,7 @@ describe('SlackMessageAdapter', function () {
     beforeEach(function () {
       this.adapter = new SlackMessageAdapter(workingSigningSecret, {
         // using a short timout to make tests finish faster
-        syncResponseTimeout: 30
+        syncResponseTimeout: 50
       });
     });
 

packages/interactive-messages/src/adapter.ts

@@ -4,6 +4,7 @@ import axios, { AxiosInstance } from 'axios';
 import isString from 'lodash.isstring';
 import isRegExp from 'lodash.isregexp';
 import isFunction from 'lodash.isfunction';
+import isPlainObject from 'lodash.isplainobject';
 import debugFactory from 'debug';
 import { ErrorCode, errorWithCode, CodedError } from './errors';
 import { createHTTPHandler } from './http-handler';
@@ -24,10 +25,10 @@ function formatMatchingConstraints<C extends AnyConstraints>(matchingConstraints
   }
 
   let ret: AnyConstraints = {};
-  if (typeof matchingConstraints === 'string' || matchingConstraints instanceof RegExp) {
-    ret.callbackId = matchingConstraints;
+  if (!isPlainObject(matchingConstraints)) {
+    ret.callbackId = matchingConstraints as string | RegExp;
   } else {
-    ret = Object.assign({}, matchingConstraints);
+    ret = Object.assign({}, matchingConstraints as C);
   }
   return ret as C;
 }
@@ -245,16 +246,15 @@ export class SlackMessageAdapter {
     callback: ActionHandler,
   ): this {
     const actionConstraints = formatMatchingConstraints(matchingConstraints);
-    const storableConstraints = Object.assign(actionConstraints, {
-      handlerType: StoredConstraintsType.Action as const,
-    });
-
     const error = validateConstraints(actionConstraints);
     if (error) {
       debug('action could not be registered: %s', error.message);
       throw error;
     }
 
+    const storableConstraints = Object.assign(actionConstraints, {
+      handlerType: StoredConstraintsType.Action as const,
+    });
     return this.registerCallback(storableConstraints, callback);
   }
 
@@ -280,17 +280,16 @@ export class SlackMessageAdapter {
     callback: OptionsHandler,
   ): this {
     const optionsConstraints = formatMatchingConstraints(matchingConstraints);
-    const storableConstraints = Object.assign(optionsConstraints, {
-      handlerType: StoredConstraintsType.Options as const,
-    });
-
     const error = validateConstraints(optionsConstraints) ||
       validateOptionsConstraints(optionsConstraints);
     if (error) {
       debug('options could not be registered: %s', error.message);
       throw error;
     }
 
+    const storableConstraints = Object.assign(optionsConstraints, {
+      handlerType: StoredConstraintsType.Options as const,
+    });
     return this.registerCallback(storableConstraints, callback);
   }
 

packages/interactive-messages/src/http-handler.ts

@@ -110,7 +110,7 @@ export function createHTTPHandler(adapter: SlackMessageAdapter): RequestListener
         if (verifyRequestSignature(adapter.signingSecret, req.headers as VerificationHeaders, rawBody)) {
           // Request signature is verified
           // Parse raw body
-          const body = parseBody(rawBody) as any;
+          const body = parseBody(rawBody);
 
           if (body.ssl_check) {
             respond({ status: 200 });