nodejs/hookshot
nodejs/hookshot
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-hookshot.git synced 2025-03-10 21:19:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix GitHub notification bugs (#173)

Browse Source 
* Fix membership prefill for config permissions

* Fix GitHub watcher not starting

* Remove defunct command "github startoauth"

* Fix notifications not starting on startup

* changelog

* Remove unused
This commit is contained in:
Will Hunt 2022-01-14 19:24:12 +00:00 committed by GitHub
parent 3e36cd7092
commit a9c29af0d6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 32 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/173.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a few issues preventing GitHub notifications from working

2
src/AdminRoom.ts
View File

@ -471,7 +471,7 @@ export class AdminRoom extends AdminRoomCommandHandler {
}
const newData = updateFn(oldData);
await this.botIntent.underlyingClient.setRoomAccountData(BRIDGE_ROOM_TYPE, this.roomId, newData);
this.emit("settings.changed", this, oldData, newData);
this.emit("settings.changed", this, newData, oldData);
this.data = newData;
return newData;
}

5
src/Bridge.ts
View File

@ -1,6 +1,6 @@
import { AdminAccountData } from "./AdminRoomCommandHandler";
import { AdminRoom, BRIDGE_ROOM_TYPE, LEGACY_BRIDGE_ROOM_TYPE } from "./AdminRoom";
import { Appservice, IAppserviceRegistration, RichRepliesPreprocessor, IRichReplyMetadata, StateEvent, PantalaimonClient, MatrixClient, IAppserviceStorageProvider, EventKind } from "matrix-bot-sdk";
import { Appservice, IAppserviceRegistration, RichRepliesPreprocessor, IRichReplyMetadata, StateEvent, PantalaimonClient, MatrixClient, EventKind } from "matrix-bot-sdk";
import { BridgeConfig, BridgePermissionLevel, GitLabInstance } from "./Config/Config";
import { BridgeWidgetApi } from "./Widgets/BridgeWidgetApi";
import { CommentProcessor } from "./CommentProcessor";
@ -83,6 +83,7 @@ export class Bridge {
public async start() {
log.info('Starting up');
await this.config.prefillMembershipCache(this.as.botClient);
if (this.config.github) {
this.github = new GithubInstance(this.config.github.auth.id, await fs.readFile(this.config.github.auth.privateKeyFile, 'utf-8'));
@ -845,7 +846,7 @@ export class Bridge {
// Make this more efficent.
if (!oldSettings.github?.notifications?.enabled && settings.github?.notifications?.enabled) {
log.info(`Notifications enabled for ${adminRoom.userId}`);
const token = await this.tokenStore.getUserToken("github", adminRoom.userId);
const token = await this.tokenStore.getGitHubToken(adminRoom.userId);
if (token) {
log.info(`Notifications enabled for ${adminRoom.userId} and token was found`);
await this.queue.push<NotificationsEnableEvent>({

5
src/Config/Config.ts
View File

@ -317,9 +317,12 @@ export class BridgeConfig {
}
public async prefillMembershipCache(client: MatrixClient) {
for(const roomEntry of this.bridgePermissions.getInterestedRooms()) {
const permissionRooms = this.bridgePermissions.getInterestedRooms();
log.info(`Prefilling room membership for permissions for ${permissionRooms.length} rooms`);
for(const roomEntry of permissionRooms) {
const membership = await client.getJoinedRoomMembers(await client.resolveRoom(roomEntry));
membership.forEach(userId => this.bridgePermissions.addMemberToCache(roomEntry, userId));
log.info(`Found ${membership.length} users for ${roomEntry}`);
}
}

6
src/Github/AdminCommands.ts
View File

@ -32,12 +32,6 @@ export class GitHubBotCommands extends AdminRoomCommandHandler {
return this.sendNotice(`To login, open ${generateGitHubOAuthUrl(this.config.github.oauth.client_id, this.config.github.oauth.redirect_uri, state)} to link your account to the bridge`);
}
@botCommand("github startoauth", {help: "Start the OAuth process with GitHub", category: "github"})
public async beginOAuth() {
// Legacy command
return this.loginCommand();
}
@botCommand("github setpersonaltoken", {help: "Set your personal access token for GitHub", requiredArgs: ['accessToken'], category: "github"})
public async setGHPersonalAccessToken(accessToken: string) {
if (!this.config.github) {

6
src/Github/GithubInstance.ts
View File

@ -1,5 +1,4 @@
import { createAppAuth } from "@octokit/auth-app";
import { createTokenAuth } from "@octokit/auth-token";
import { Octokit } from "@octokit/rest";
import LogWrapper from "../LogWrapper";
import { DiscussionQLResponse, DiscussionQL } from "./Discussion";
@ -44,10 +43,7 @@ export class GithubInstance {
public static createUserOctokit(token: string) {
return new Octokit({
// XXX: A recent release of octokit (rest/auth-token?) broke passing in the token
// as an auth parameter. For now we can just do this.
authStrategy: () => createTokenAuth(token),
auth: null,
auth: token,
userAgent: USER_AGENT,
});
}

30
src/Notifications/GitHubWatcher.ts
View File

@ -1,10 +1,9 @@
import { Octokit } from "@octokit/rest";
import { Octokit, RestEndpointMethodTypes } from "@octokit/rest";
import { EventEmitter } from "events";
import { GithubInstance } from "../Github/GithubInstance";
import LogWrapper from "../LogWrapper";
import { NotificationWatcherTask } from "./NotificationWatcherTask";
import { RequestError } from "@octokit/request-error";
import { GitHubUserNotification } from "../Github/Types";
import { OctokitResponse } from "@octokit/types";
import Metrics from "../Metrics";
const log = new LogWrapper("GitHubWatcher");
@ -12,6 +11,8 @@ const log = new LogWrapper("GitHubWatcher");
const GH_API_THRESHOLD = 50;
const GH_API_RETRY_IN = 1000 * 60;
type GitHubUserNotification = RestEndpointMethodTypes["activity"]["listNotificationsForAuthenticatedUser"]["response"];
export class GitHubWatcher extends EventEmitter implements NotificationWatcherTask {
private static apiFailureCount = 0;
private static globalRetryIn = 0;
@ -76,11 +77,9 @@ export class GitHubWatcher extends EventEmitter implements NotificationWatcherTa
}
log.debug(`Getting notifications for ${this.userId} ${this.lastReadTs}`);
const since = this.lastReadTs !== 0 ? `&since=${new Date(this.lastReadTs).toISOString()}`: "";
let response: OctokitResponse<GitHubUserNotification[]>;
let response: GitHubUserNotification;
try {
response = await this.octoKit.request(
`/notifications?participating=${this.participating}${since}`,
);
response = await this.octoKit.activity.listNotificationsForAuthenticatedUser({since, participating: this.participating});
Metrics.notificationsServiceUp.set({service: "github"}, 1);
// We were succesful, clear any timeouts.
GitHubWatcher.globalRetryIn = 0;
@ -96,17 +95,18 @@ export class GitHubWatcher extends EventEmitter implements NotificationWatcherTa
log.info(`Got ${response.data.length} notifications for ${this.userId}`);
}
for (const rawEvent of response.data) {
const ev = rawEvent as any;
try {
if (rawEvent.subject.url) {
const res = await this.octoKit.request(rawEvent.subject.url);
rawEvent.subject.url_data = res.data;
ev.url_data = res.data;
}
if (rawEvent.subject.latest_comment_url) {
const res = await this.octoKit.request(rawEvent.subject.latest_comment_url);
rawEvent.subject.latest_comment_url_data = res.data;
ev.latest_comment_url_data = res.data;
}
if (rawEvent.reason === "review_requested") {
if (!rawEvent.subject.url_data?.number) {
if (!ev.url_data?.number) {
log.warn("review_requested was missing subject.url_data.number");
continue;
}
@ -114,14 +114,14 @@ export class GitHubWatcher extends EventEmitter implements NotificationWatcherTa
log.warn("review_requested was missing repository.owner");
continue;
}
rawEvent.subject.requested_reviewers = (await this.octoKit.pulls.listRequestedReviewers({
pull_number: rawEvent.subject.url_data.number,
ev.requested_reviewers = (await this.octoKit.pulls.listRequestedReviewers({
pull_number: ev.url_data.number,
owner: rawEvent.repository.owner.login,
repo: rawEvent.repository.name,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
})).data as any;
rawEvent.subject.reviews = (await this.octoKit.pulls.listReviews({
pull_number: rawEvent.subject.url_data.number,
ev.reviews = (await this.octoKit.pulls.listReviews({
pull_number: ev.url_data.number,
owner: rawEvent.repository.owner.login,
repo: rawEvent.repository.name,
})).data;
@ -130,13 +130,13 @@ export class GitHubWatcher extends EventEmitter implements NotificationWatcherTa
log.warn(`Failed to pre-process ${rawEvent.id}: ${ex}`);
// We still push
}
log.debug(`Pushing ${rawEvent.id}`);
log.debug(`Pushing ${ev.id}`);
Metrics.notificationsPush.inc({service: "github"});
this.emit("new_events", {
eventName: "notifications.user.events",
data: {
roomId: this.roomId,
events: [rawEvent],
events: [ev],
lastReadTs: this.lastReadTs,
},
sender: "GithubWebhooks",

10
src/UserTokenStore.ts
View File

@ -98,8 +98,7 @@ export class UserTokenStore {
return null;
}
public async getOctokitForUser(userId: string) {
// TODO: Move this somewhere else.
public async getGitHubToken(userId: string) {
const storeTokenResponse = await this.getUserToken("github", userId);
if (!storeTokenResponse) {
return null;
@ -139,7 +138,12 @@ export class UserTokenStore {
throw Error('Token is expired, cannot refresh');
}
}
return GithubInstance.createUserOctokit(senderToken.access_token);
return senderToken.access_token;
}
public async getOctokitForUser(userId: string) {
const res = await this.getGitHubToken(userId);
return res ? GithubInstance.createUserOctokit(res) : null;
}
public async getGitLabForUser(userId: string, instanceUrl: string) {