Skip to content

Commit

Permalink
feat: update @octokit/webhooks to v9 (#1559)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: remove '*' event
BREAKING CHANGE: app.webhooks.middleware has been removed in `@octokit/webhooks` v9
BREAKING CHANGE: removes the `webhookPath` option on `new Probot({})` for the webhooks middleware

Co-authored-by: wolfy1339 <webmaster@wolfy1339.com>
  • Loading branch information
gr2m and wolfy1339 committed Jun 24, 2021
1 parent be1711e commit 4b3ae0e
Show file tree
Hide file tree
Showing 21 changed files with 970 additions and 1,152 deletions.
1,632 changes: 713 additions & 919 deletions package-lock.json

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion package.json
Expand Up @@ -42,7 +42,7 @@
"@octokit/plugin-retry": "^3.0.6",
"@octokit/plugin-throttling": "^3.3.4",
"@octokit/types": "^6.1.1",
"@octokit/webhooks": "7.21.0",
"@octokit/webhooks": "^9.8.4",
"@probot/get-private-key": "^1.1.0",
"@probot/octokit-plugin-config": "^1.0.0",
"@probot/pino": "^2.2.0",
Expand Down Expand Up @@ -72,6 +72,9 @@
"uuid": "^8.3.2"
},
"devDependencies": {
"@octokit/webhooks-examples": "^4.0.0",
"@octokit/webhooks-methods": "^2.0.0",
"@octokit/webhooks-types": "^4.0.0",
"@tsconfig/node10": "^1.0.7",
"@types/eventsource": "^1.1.5",
"@types/jest": "^26.0.18",
Expand Down
49 changes: 14 additions & 35 deletions src/context.ts
@@ -1,43 +1,17 @@
import path from "path";

import { EventPayloads, WebhookEvent } from "@octokit/webhooks";
import { EmitterWebhookEvent as WebhookEvent } from "@octokit/webhooks";
import merge from "deepmerge";

import type { Logger } from "pino";

import { ProbotOctokit } from "./octokit/probot-octokit";
import { aliasLog } from "./helpers/alias-log";
import { DeprecatedLogger } from "./types";
import { WebhookEvents } from "@octokit/webhooks";
import { EmitterWebhookEventName as WebhookEvents } from "@octokit/webhooks/dist-types/types";

export type MergeOptions = merge.Options;

export interface WebhookPayloadWithRepository {
[key: string]: any;
repository?: EventPayloads.PayloadRepository;
issue?: {
[key: string]: any;
number: number;
html_url?: string;
body?: string;
};
pull_request?: {
[key: string]: any;
number: number;
html_url?: string;
body?: string;
};
sender?: {
[key: string]: any;
type: string;
};
action?: string;
installation?: {
id: number;
[key: string]: any;
};
}

/**
* The context of the event that was triggered, including the payload and
* helpers for extracting information can be passed to GitHub API calls.
Expand All @@ -54,11 +28,10 @@ export interface WebhookPayloadWithRepository {
* @property {payload} payload - The webhook event payload
* @property {log} log - A pino instance
*/
export class Context<E extends WebhookPayloadWithRepository = any>
implements WebhookEvent<E> {
export class Context<E extends WebhookEvents = WebhookEvents> {
public name: WebhookEvents;
public id: string;
public payload: E;
public payload: WebhookEvent<E>["payload"];

public octokit: InstanceType<typeof ProbotOctokit>;
public log: DeprecatedLogger;
Expand Down Expand Up @@ -89,6 +62,7 @@ export class Context<E extends WebhookPayloadWithRepository = any>
*
*/
public repo<T>(object?: T) {
// @ts-ignore `repository` is not always present in this.payload
const repo = this.payload.repository;

if (!repo) {
Expand All @@ -99,7 +73,7 @@ export class Context<E extends WebhookPayloadWithRepository = any>

return Object.assign(
{
owner: repo.owner.login || repo.owner.name!,
owner: repo.owner.login || repo.owner.name,
repo: repo.name,
},
object
Expand All @@ -119,10 +93,12 @@ export class Context<E extends WebhookPayloadWithRepository = any>
* @param object - Params to be merged with the issue params.
*/
public issue<T>(object?: T) {
const payload = this.payload;
return Object.assign(
{
issue_number: (payload.issue || payload.pull_request || payload).number,
issue_number:
// @ts-ignore - this.payload may not have `issue` or `pull_request` keys
(this.payload.issue || this.payload.pull_request || this.payload)
.number,
},
this.repo(object)
);
Expand All @@ -144,6 +120,7 @@ export class Context<E extends WebhookPayloadWithRepository = any>
const payload = this.payload;
return Object.assign(
{
// @ts-ignore - this.payload may not have `issue` or `pull_request` keys
pull_number: (payload.issue || payload.pull_request || payload).number,
},
this.repo(object)
Expand All @@ -155,7 +132,9 @@ export class Context<E extends WebhookPayloadWithRepository = any>
* @type {boolean}
*/
get isBot() {
return this.payload.sender!.type === "Bot";
// @ts-expect-error - `sender` key is currently not present in all events
// see https://github.com/octokit/webhooks/issues/510
return this.payload.sender.type === "Bot";
}

/**
Expand Down
7 changes: 5 additions & 2 deletions src/create-node-middleware.ts
@@ -1,13 +1,16 @@
import { RequestListener } from "http";
import { createNodeMiddleware as createWebbhooksMiddleware } from "@octokit/webhooks";

import { ApplicationFunction } from "./types";
import { MiddlewareOptions } from "./types";

export function createNodeMiddleware(
appFn: ApplicationFunction,
{ probot }: MiddlewareOptions
{ probot, webhooksPath }: MiddlewareOptions
): RequestListener {
probot.load(appFn);

return probot.webhooks.middleware;
return createWebbhooksMiddleware(probot.webhooks, {
path: webhooksPath || "/",
});
}
15 changes: 9 additions & 6 deletions src/helpers/get-error-handler.ts
@@ -1,18 +1,21 @@
import type { Logger } from "pino";
import { WebhookError, WebhookEvent } from "@octokit/webhooks";
import {
WebhookError,
EmitterWebhookEvent as WebhookEvent,
} from "@octokit/webhooks";

export function getErrorHandler(log: Logger) {
return (error: Error) => {
const errors = (error.name === "AggregateError"
? error
: [error]) as WebhookError[];
const errors = (
error.name === "AggregateError" ? error : [error]
) as WebhookError[];

const event = (error as any).event as WebhookEvent<any>;
const event = (error as any).event as WebhookEvent;

for (const error of errors) {
const errMessage = (error.message || "").toLowerCase();

if (errMessage.includes("x-hub-signature")) {
if (errMessage.includes("x-hub-signature-256")) {
log.error(
error,
"Go to https://github.com/settings/apps/YOUR_APP and verify that the Webhook secret matches the value of the WEBHOOK_SECRET environment variable."
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
@@ -1,6 +1,6 @@
import { Logger } from "pino";

import { Context, WebhookPayloadWithRepository } from "./context";
import { Context } from "./context";
import { Options, ApplicationFunctionOptions } from "./types";
import { Probot } from "./probot";
import { Server } from "./server/server";
Expand All @@ -21,4 +21,4 @@ export {
};

/** NOTE: exported types might change at any point in time */
export { Options, WebhookPayloadWithRepository, ApplicationFunctionOptions };
export { Options, ApplicationFunctionOptions };
12 changes: 7 additions & 5 deletions src/octokit/get-webhooks.ts
@@ -1,15 +1,17 @@
import { WebhookEvent, Webhooks } from "@octokit/webhooks";
import { Webhooks } from "@octokit/webhooks";

import { State } from "../types";
import { getErrorHandler } from "../helpers/get-error-handler";
import { webhookTransform } from "./octokit-webhooks-transform";

import { Context } from "../context";
// import { Context } from "../context";

export function getWebhooks(state: State) {
const webhooks = new Webhooks<WebhookEvent, Context>({
path: state.webhooks.path,
secret: state.webhooks.secret,
// TODO: This should be webhooks = new Webhooks<Context>({...}) but fails with
// > The context of the event that was triggered, including the payload and
// helpers for extracting information can be passed to GitHub API calls
const webhooks = new Webhooks({
secret: state.webhooks.secret!,
transform: webhookTransform.bind(null, state),
});
webhooks.onError(getErrorHandler(state.log));
Expand Down
13 changes: 4 additions & 9 deletions src/octokit/octokit-plugin-probot-request-logging.ts
Expand Up @@ -4,12 +4,8 @@ import type { Octokit } from "@octokit/core";
export function probotRequestLogging(octokit: Octokit) {
octokit.hook.error("request", (error, options) => {
if ("status" in error) {
const {
method,
url,
request,
...params
} = octokit.request.endpoint.parse(options);
const { method, url, request, ...params } =
octokit.request.endpoint.parse(options);
const msg = `GitHub request: ${method} ${url} - ${error.status}`;

// @ts-expect-error log.debug is a pino log method and accepts a fields object
Expand All @@ -20,9 +16,8 @@ export function probotRequestLogging(octokit: Octokit) {
});

octokit.hook.after("request", (result, options) => {
const { method, url, request, ...params } = octokit.request.endpoint.parse(
options
);
const { method, url, request, ...params } =
octokit.request.endpoint.parse(options);
const msg = `GitHub request: ${method} ${url} - ${result.status}`;

// @ts-ignore log.debug is a pino log method and accepts a fields object
Expand Down
2 changes: 1 addition & 1 deletion src/octokit/octokit-webhooks-transform.ts
@@ -1,4 +1,4 @@
import { WebhookEvent } from "@octokit/webhooks";
import { EmitterWebhookEvent as WebhookEvent } from "@octokit/webhooks";

import { Context } from "../context";
import { State } from "../types";
Expand Down
17 changes: 2 additions & 15 deletions src/probot.ts
@@ -1,6 +1,6 @@
import LRUCache from "lru-cache";
import { Logger } from "pino";
import { WebhookEvent } from "@octokit/webhooks";
import { EmitterWebhookEvent as WebhookEvent } from "@octokit/webhooks";

import { aliasLog } from "./helpers/alias-log";
import { auth } from "./auth";
Expand Down Expand Up @@ -46,7 +46,6 @@ export class Probot {
private state: State;

constructor(options: Options = {}) {
options.webhookPath = options.webhookPath || "/";
options.secret = options.secret || "development";

let level = options.logLevel;
Expand Down Expand Up @@ -81,7 +80,6 @@ export class Probot {
Octokit,
octokit,
webhooks: {
path: options.webhookPath,
secret: options.secret,
},
appId: Number(options.appId),
Expand All @@ -94,18 +92,7 @@ export class Probot {

this.webhooks = getWebhooks(this.state);

this.on = (eventNameOrNames, callback) => {
if (eventNameOrNames === "*") {
this.log.warn(
`[probot] Using the "*" event with the regular app.on() function is deprecated. Please use the app.webhooks.onAny() method instead`
);
// @ts-ignore this.webhooks.on("*") is deprecated
return this.webhooks.onAny(callback);
}

return this.webhooks.on(eventNameOrNames, callback);
};

this.on = this.webhooks.on;
this.onAny = this.webhooks.onAny;
this.onError = this.webhooks.onError;

Expand Down
5 changes: 4 additions & 1 deletion src/server/server.ts
Expand Up @@ -3,6 +3,7 @@ import { Server as HttpServer } from "http";
import express, { Application, Router } from "express";
import { join } from "path";
import { Logger } from "pino";
import { createNodeMiddleware as createWebhooksMiddleware } from "@octokit/webhooks";

import { getLog } from "../helpers/get-log";
import { getLoggingMiddleware } from "./logging-middleware";
Expand Down Expand Up @@ -49,7 +50,9 @@ export class Server {
);
this.expressApp.use(
this.state.webhookPath,
this.probotApp.webhooks.middleware
createWebhooksMiddleware(this.probotApp.webhooks, {
path: "/",
})
);

this.expressApp.set("view engine", "hbs");
Expand Down
13 changes: 6 additions & 7 deletions src/types.ts
@@ -1,5 +1,8 @@
import express from "express";
import { WebhookEvent, Webhooks } from "@octokit/webhooks";
import {
EmitterWebhookEvent as WebhookEvent,
Webhooks,
} from "@octokit/webhooks";
import LRUCache from "lru-cache";
import Redis from "ioredis";

Expand All @@ -18,7 +21,6 @@ export interface Options {
log?: Logger;
redisConfig?: Redis.RedisOptions | string;
secret?: string;
webhookPath?: string;
logLevel?: "trace" | "debug" | "info" | "warn" | "error" | "fatal";
logMessageKey?: string;
port?: number;
Expand All @@ -35,18 +37,14 @@ export type State = {
octokit: InstanceType<typeof ProbotOctokit>;
cache?: LRUCache<number, string>;
webhooks: {
path?: string;
secret?: string;
};
port?: number;
host?: string;
baseUrl?: string;
};

export type ProbotWebhooks = Webhooks<
WebhookEvent,
Omit<Context, keyof WebhookEvent>
>;
export type ProbotWebhooks = Webhooks<Omit<Context, keyof WebhookEvent>>;

export type DeprecatedLogger = LogFn & Logger;

Expand All @@ -70,5 +68,6 @@ export type ServerOptions = {

export type MiddlewareOptions = {
probot: Probot;
webhooksPath?: string;
[key: string]: unknown;
};

0 comments on commit 4b3ae0e

Please sign in to comment.