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.

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

feat: update @octokit/webhooks to v9 #1559

Merged
merged 35 commits into from Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1e6eb74
feat: upgrade to `@octokit/webhooks` v9
wolfy1339 Apr 27, 2021
072e675
fix: remove path option when creating the `Webhooks` class
wolfy1339 Apr 27, 2021
677bbad
fix: use the new middleware from `@octokit/webhooks`
wolfy1339 Apr 27, 2021
c59bbdc
refactor(tests): simplify declaration of `event`
wolfy1339 Apr 27, 2021
8b576d8
refactor(tests): move declaration of `event` variable to it's own `be…
wolfy1339 Apr 27, 2021
b6b2326
tests: remove the `*` event
wolfy1339 Apr 27, 2021
663f619
fix: remove wrapper around `Webooks#on`
wolfy1339 Apr 27, 2021
dbec054
fix: clean up imports in `src/context.ts`
wolfy1339 Apr 27, 2021
7502f94
fix: update `getErrorHandler()` imports and typecasting
wolfy1339 Apr 27, 2021
2df7788
lint: run prettier
wolfy1339 Apr 27, 2021
b0fe0c2
tests: fixups
wolfy1339 Apr 28, 2021
8d67e8a
build: bump `@octokit/webhooks-examples`
wolfy1339 Apr 28, 2021
40b7645
fix: comment out `implement` in `Context` for now
wolfy1339 Apr 28, 2021
f020b6c
fix: pass path down to the webhooks middleware
wolfy1339 Apr 28, 2021
baa050d
fix: add missing parameter name to JSDoc comment
wolfy1339 Apr 28, 2021
c8ea489
style: prettier
gr2m Apr 29, 2021
b9c545e
test: fix context tests
gr2m Apr 29, 2021
19cd186
fix: `x-hub-signature-256` header is now required
wolfy1339 Apr 29, 2021
ac66195
refactor: remove un-needed `path` option
wolfy1339 Apr 29, 2021
e4aea01
tests: change test name to reflect x-hub-signature-256 requirement
wolfy1339 Apr 29, 2021
de6c747
feat: introduce new `webhooksPath` option for the middleware
wolfy1339 Apr 29, 2021
b70c486
test: remove unhandled promise rejection error in `test/create-node-m…
gr2m Apr 29, 2021
cf57401
test: adapt server test for latest `@octokit/webhooks`. It is techinc…
gr2m Apr 29, 2021
38e59e2
build(package): lock file
gr2m Apr 29, 2021
ff4a986
build(deps): Unpin `@octokit/webhooks`, bump `@octokit/webhooks-examp…
wolfy1339 Apr 29, 2021
13b41cb
build(deps): bump `@octokit/webhooks*` packages to latest
gr2m Jun 18, 2021
28144bc
build(package): lock file
gr2m Jun 18, 2021
1d49aa1
WIP: workaround "Expression produces a union type that is too complex…
gr2m Jun 19, 2021
ce64dd3
build(package): bump `@octokit/webhooks` to `^9.8.4`
gr2m Jun 22, 2021
49dfe2c
build(package): lock file
gr2m Jun 22, 2021
e46459c
style: prettier
gr2m Jun 22, 2021
293e903
test: pass string payload body instead of object
gr2m Jun 22, 2021
26574c9
docs: add comment about `new Webhooks<Context>({...})`
gr2m Jun 22, 2021
5774187
test: skip run test that was skipped before
gr2m Jun 22, 2021
4070737
fix: mount `@octokit/webhooks` middleware to the webhooks path option
gr2m Jun 22, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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({
wolfy1339 marked this conversation as resolved.
Show resolved Hide resolved
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;
};