From 85a7741809d4f89e78eb3023431491159b74aaf7 Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 16:44:35 -0400 Subject: [PATCH 1/9] feat: Add onAny and onEvent functions Fixes #304 --- src/event-handler/index.ts | 2 ++ src/index.ts | 5 +++++ test/integration/event-handler-test.ts | 11 +++++++++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/event-handler/index.ts b/src/event-handler/index.ts index 7f2d6e32..59966cd3 100644 --- a/src/event-handler/index.ts +++ b/src/event-handler/index.ts @@ -14,6 +14,8 @@ export function createEventHandler(options: Options) { return { on: on.bind(null, state), + onAny: on.bind(null, state).bind(null, "*"), + onError: on.bind(null, state).bind(null, "error"), removeListener: removeListener.bind(null, state), receive: receive.bind(null, state), }; diff --git a/src/index.ts b/src/index.ts index 44fb6d6e..6ec2fb42 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,7 @@ import { State, WebhookEvent, WebhookError, + WebhookEventHandlerError, HandlerFunction, } from "./types"; import { IncomingMessage, ServerResponse } from "http"; @@ -22,6 +23,8 @@ class Webhooks { event: E | E[], callback: HandlerFunction ) => void; + public onAny: (callback: (event: WebhookEvent) => any) => void; + public onError: (callback: (event: WebhookEventHandlerError) => any) => void; public removeListener: ( event: E | E[], callback: HandlerFunction @@ -55,6 +58,8 @@ class Webhooks { this.sign = sign.bind(null, options.secret); this.verify = verify.bind(null, options.secret); this.on = state.eventHandler.on; + this.onAny = state.eventHandler.onAny; + this.onError = state.eventHandler.onError; this.removeListener = state.eventHandler.removeListener; this.receive = state.eventHandler.receive; this.middleware = middleware.bind(null, state); diff --git a/test/integration/event-handler-test.ts b/test/integration/event-handler-test.ts index 10c2996d..52c9a243 100644 --- a/test/integration/event-handler-test.ts +++ b/test/integration/event-handler-test.ts @@ -1,7 +1,7 @@ import { createEventHandler } from "../../src/event-handler"; import pushEventPayload from "../fixtures/push-payload.json"; import installationCreatedPayload from "../fixtures/installation-created-payload.json"; -import { WebhookError, WebhookEvent } from "../../src/types"; +import { WebhookEvent, WebhookEventHandlerError } from "../../src/types"; test("events", (done) => { const eventHandler = createEventHandler({}); @@ -38,6 +38,7 @@ test("events", (done) => { eventHandler.on("installation", hook5); eventHandler.on("installation.created", hook6); eventHandler.on("*", hook7); + eventHandler.onAny(hook7); eventHandler.removeListener("push", hook3); eventHandler.removeListener(["push"], hook4); @@ -68,7 +69,13 @@ test("events", (done) => { "* (installation)", ]); - eventHandler.on("error", (error: WebhookError) => { + eventHandler.on("error", (error: WebhookEventHandlerError) => { + expect(error.event.payload).toBeTruthy(); + // t.pass("error event triggered"); + expect(error.message).toMatch(/oops/); + }); + + eventHandler.onError((error: WebhookEventHandlerError) => { expect(error.event.payload).toBeTruthy(); // t.pass("error event triggered"); expect(error.message).toMatch(/oops/); From ef7b19e0397e3050ea0cf3e79df5d9582d9f4d9e Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 16:57:22 -0400 Subject: [PATCH 2/9] tests: Add the new functions to typescript-validate --- test/typescript-validate.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/typescript-validate.ts b/test/typescript-validate.ts index f3a90891..9af72de3 100644 --- a/test/typescript-validate.ts +++ b/test/typescript-validate.ts @@ -69,12 +69,19 @@ export default async function () { verify("randomSecret", {}, "randomSignature"); + // This is deprecated usage webhooks.on("*", ({ id, name, payload }) => { console.log(name, "event received"); const sig = webhooks.sign(payload); webhooks.verify(payload, sig); }); + webhooks.onAny(({ id, name, payload }) => { + console.log(name, "event received"); + const sig = webhooks.sign(payload); + webhooks.verify(payload, sig); + }); + webhooks.on("check_run.completed", () => {}); webhooks.on( @@ -124,6 +131,7 @@ export default async function () { console.log(event.foo); }); + // This is deprecated usage webhooks.on("error", (error) => { console.log(error.event.name); const [firstError] = Array.from(error); @@ -132,6 +140,14 @@ export default async function () { console.log(firstError.request); }); + webhooks.onError((error) => { + console.log(error.event.name); + const [firstError] = Array.from(error); + console.log(firstError.status); + console.log(firstError.headers); + console.log(firstError.request); + }); + createServer(webhooks.middleware).listen(3000); } From 3f73f68423c2506a0572ae2e8233e3eaf58e4753 Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 17:37:10 -0400 Subject: [PATCH 3/9] deprecate the '*' and 'any' events --- src/event-handler/on.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/event-handler/on.ts b/src/event-handler/on.ts index bd3d0d98..4854befc 100644 --- a/src/event-handler/on.ts +++ b/src/event-handler/on.ts @@ -20,6 +20,14 @@ export function receiverOn( ); } + if (webhookNameOrNames === "*" || webhookNameOrNames === "error") { + console.warn( + `Using the "${webhookNameOrNames}" event with the regular Webhooks.on() function is deprecated. Please use either the Webhooks.on${ + webhookNameOrNames.charAt(0).toUpperCase() + webhookNameOrNames.slice(1) + } method instead` + ); + } + if (!state.hooks[webhookNameOrNames]) { state.hooks[webhookNameOrNames] = []; } From dfa19dd89dec17bcde3e8f710578fa1a94379a32 Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 17:44:43 -0400 Subject: [PATCH 4/9] Move the onAny and onError methods to their own functions --- src/event-handler/index.ts | 10 +++++++--- src/event-handler/on.ts | 31 ++++++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/event-handler/index.ts b/src/event-handler/index.ts index 59966cd3..25023ea5 100644 --- a/src/event-handler/index.ts +++ b/src/event-handler/index.ts @@ -1,4 +1,8 @@ -import { receiverOn as on } from "./on"; +import { + receiverOn as on, + receiverOnAny as onAny, + receiverOnError as onError, +} from "./on"; import { receiverHandle as receive } from "./receive"; import { removeListener } from "./remove-listener"; import { Options, State } from "../types"; @@ -14,8 +18,8 @@ export function createEventHandler(options: Options) { return { on: on.bind(null, state), - onAny: on.bind(null, state).bind(null, "*"), - onError: on.bind(null, state).bind(null, "error"), + onAny: onAny.bind(null, state), + onError: onError.bind(null, state), removeListener: removeListener.bind(null, state), receive: receive.bind(null, state), }; diff --git a/src/event-handler/on.ts b/src/event-handler/on.ts index 4854befc..f989445a 100644 --- a/src/event-handler/on.ts +++ b/src/event-handler/on.ts @@ -1,7 +1,18 @@ import { WebhookEvents } from "../generated/get-webhook-payload-type-from-event"; import { webhookNames } from "../generated/webhook-names"; -import { State } from "../types"; +import { State, WebhookEvent, WebhookEventHandlerError } from "../types"; +function handleEventHandlers( + state: State, + webhookName: WebhookEvents, + handler: Function +) { + if (!state.hooks[webhookName]) { + state.hooks[webhookName] = []; + } + + state.hooks[webhookName].push(handler); +} export function receiverOn( state: State, webhookNameOrNames: WebhookEvents | WebhookEvents[], @@ -28,9 +39,19 @@ export function receiverOn( ); } - if (!state.hooks[webhookNameOrNames]) { - state.hooks[webhookNameOrNames] = []; - } + handleEventHandlers(state, webhookNameOrNames, handler); +} - state.hooks[webhookNameOrNames].push(handler); +export function receiverOnAny( + state: State, + handler: (event: WebhookEvent) => any +) { + handleEventHandlers(state, "*", handler); +} + +export function receiverOnError( + state: State, + handler: (event: WebhookEventHandlerError) => any +) { + handleEventHandlers(state, "error", handler); } From cec16a8eef7ab03143b050cda8563eab4ca6afcd Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 18:04:22 -0400 Subject: [PATCH 5/9] Fix error message for the '*' event --- src/event-handler/on.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/event-handler/on.ts b/src/event-handler/on.ts index f989445a..e6bf04b6 100644 --- a/src/event-handler/on.ts +++ b/src/event-handler/on.ts @@ -32,9 +32,10 @@ export function receiverOn( } if (webhookNameOrNames === "*" || webhookNameOrNames === "error") { + const webhookName = webhookNameOrNames === "*" ? "any" : webhookNameOrNames; console.warn( `Using the "${webhookNameOrNames}" event with the regular Webhooks.on() function is deprecated. Please use either the Webhooks.on${ - webhookNameOrNames.charAt(0).toUpperCase() + webhookNameOrNames.slice(1) + webhookName.charAt(0).toUpperCase() + webhookName.slice(1) } method instead` ); } From d62003eb3c5bdd7df3bee91cf3f85b15d5976e96 Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 18:04:51 -0400 Subject: [PATCH 6/9] Fix wording of error message --- src/event-handler/on.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/event-handler/on.ts b/src/event-handler/on.ts index e6bf04b6..cf7e0ddf 100644 --- a/src/event-handler/on.ts +++ b/src/event-handler/on.ts @@ -34,9 +34,9 @@ export function receiverOn( if (webhookNameOrNames === "*" || webhookNameOrNames === "error") { const webhookName = webhookNameOrNames === "*" ? "any" : webhookNameOrNames; console.warn( - `Using the "${webhookNameOrNames}" event with the regular Webhooks.on() function is deprecated. Please use either the Webhooks.on${ + `Using the "${webhookNameOrNames}" event with the regular Webhooks.on() function is deprecated. Please use the Webhooks.on${ webhookName.charAt(0).toUpperCase() + webhookName.slice(1) - } method instead` + }() method instead` ); } From 59b4146162d796b80748290a10f274500a4a62d0 Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 18:05:15 -0400 Subject: [PATCH 7/9] Add unit test to test the deprecation --- test/unit/event-handler-on-test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/unit/event-handler-on-test.ts b/test/unit/event-handler-on-test.ts index 243149fe..c94738fd 100644 --- a/test/unit/event-handler-on-test.ts +++ b/test/unit/event-handler-on-test.ts @@ -20,3 +20,21 @@ test("receiver.on with invalid event name", () => { simple.restore(); }); + +test("receiver.on with event name of '*' logs deprecation notice", () => { + simple.mock(console, "warn").callFn(function () {}); + receiverOn(state, "*", noop); + expect((console.warn as simple.Stub).callCount).toBe(1); + expect((console.warn as simple.Stub).lastCall.arg).toBe( + 'Using the "*" event with the regular Webhooks.on() function is deprecated. Please use the Webhooks.onAny() method instead' + ); +}); + +test("receiver.on with event name of 'error' logs deprecation notice", () => { + simple.mock(console, "warn").callFn(function () {}); + receiverOn(state, "error", noop); + expect((console.warn as simple.Stub).callCount).toBe(1); + expect((console.warn as simple.Stub).lastCall.arg).toBe( + 'Using the "error" event with the regular Webhooks.on() function is deprecated. Please use the Webhooks.onError() method instead' + ); +}); From 99d7d41e6142a5d08b5655f80f277e5b57a9de24 Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Tue, 13 Oct 2020 18:15:41 -0400 Subject: [PATCH 8/9] Also use the onError method in the server integration test --- test/integration/server-test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/server-test.ts b/test/integration/server-test.ts index 2a314953..9c4d2977 100644 --- a/test/integration/server-test.ts +++ b/test/integration/server-test.ts @@ -142,7 +142,7 @@ describe("server-test", () => { }); const server = http.createServer(api.middleware); const errorHandler = jest.fn(); - api.on("error", errorHandler); + api.onError(errorHandler); promisify(server.listen.bind(server))(availablePort) @@ -177,7 +177,7 @@ describe("server-test", () => { }); const server = http.createServer(api.middleware); const errorHandler = jest.fn(); - api.on("error", errorHandler); + api.onError(errorHandler); promisify(server.listen.bind(server))(availablePort) From 8d756488b6cf1ccb155bc5a7395bc9d321c05962 Mon Sep 17 00:00:00 2001 From: wolfy1339 Date: Wed, 14 Oct 2020 16:31:32 -0400 Subject: [PATCH 9/9] Remove main usage tests for deprecated usage --- test/integration/event-handler-test.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/integration/event-handler-test.ts b/test/integration/event-handler-test.ts index 52c9a243..43084af0 100644 --- a/test/integration/event-handler-test.ts +++ b/test/integration/event-handler-test.ts @@ -37,7 +37,6 @@ test("events", (done) => { eventHandler.on(["push"], hook4); eventHandler.on("installation", hook5); eventHandler.on("installation.created", hook6); - eventHandler.on("*", hook7); eventHandler.onAny(hook7); eventHandler.removeListener("push", hook3); @@ -69,12 +68,6 @@ test("events", (done) => { "* (installation)", ]); - eventHandler.on("error", (error: WebhookEventHandlerError) => { - expect(error.event.payload).toBeTruthy(); - // t.pass("error event triggered"); - expect(error.message).toMatch(/oops/); - }); - eventHandler.onError((error: WebhookEventHandlerError) => { expect(error.event.payload).toBeTruthy(); // t.pass("error event triggered");