From 9d841697b2ed047352b7d11ad2ea11331a70cbaa Mon Sep 17 00:00:00 2001 From: gnuxie Date: Tue, 5 Sep 2023 18:05:25 +0100 Subject: [PATCH] Rework RoomUpdateErrors to be CommandErrors or CommandExceptions https://github.com/Gnuxie/Draupnir/pull/93/ --- src/ProtectedRoomsSet.ts | 53 ++++++++++++++-------------- src/models/RoomUpdateError.tsx | 46 ++++++++++++++++++------ src/protections/BanPropagation.tsx | 18 +++++----- src/protections/ProtectionManager.ts | 51 ++++++++------------------ src/queues/EventRedactionQueue.ts | 28 +++++++-------- 5 files changed, 97 insertions(+), 99 deletions(-) diff --git a/src/ProtectedRoomsSet.ts b/src/ProtectedRoomsSet.ts index 6d2cfad0..0bddbd3e 100644 --- a/src/ProtectedRoomsSet.ts +++ b/src/ProtectedRoomsSet.ts @@ -25,14 +25,15 @@ limitations under the License. * are NOT distributed, contributed, committed, or licensed under the Apache License. */ -import { LogLevel, LogService, MatrixGlob, UserID } from "matrix-bot-sdk"; +import { LogLevel, MatrixGlob, UserID } from "matrix-bot-sdk"; +import { CommandExceptionKind } from "./commands/interface-manager/CommandException"; import { IConfig } from "./config"; import ManagementRoomOutput from "./ManagementRoomOutput"; import { MatrixSendClient } from "./MatrixEmitter"; import AccessControlUnit, { Access } from "./models/AccessControlUnit"; import { RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/ListRule"; import PolicyList, { ListRuleChange, Revision } from "./models/PolicyList"; -import { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION, printActionResult, RoomUpdateError } from "./models/RoomUpdateError"; +import { printActionResult, IRoomUpdateError, RoomUpdateException } from "./models/RoomUpdateError"; import { ProtectionManager } from "./protections/ProtectionManager"; import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQueue"; import { ProtectedRoomActivityTracker } from "./queues/ProtectedRoomActivityTracker"; @@ -181,7 +182,7 @@ export class ProtectedRoomsSet { * @param roomId Limit processing to one room only, otherwise process redactions for all rooms. * @returns The list of errors encountered, for reporting to the management room. */ - public async processRedactionQueue(roomId?: string): Promise { + public async processRedactionQueue(roomId?: string): Promise { return await this.eventRedactionQueue.process(this.client, this.managementRoomOutput, roomId); } @@ -290,7 +291,7 @@ export class ProtectedRoomsSet { * @param {string[]} roomIds The room IDs to apply the ACLs in. * @param {Mjolnir} mjolnir The Mjolnir client to apply the ACLs with. */ - private async applyServerAcls(lists: PolicyList[], roomIds: string[]): Promise { + private async applyServerAcls(lists: PolicyList[], roomIds: string[]): Promise { // we need to provide mutual exclusion so that we do not have requests updating the m.room.server_acl event // finish out of order and therefore leave the room out of sync with the policy lists. if (this.config.disableServerACL) { @@ -303,7 +304,7 @@ export class ProtectedRoomsSet { }); } - private async _applyServerAcls(lists: PolicyList[], roomIds: string[]): Promise { + private async _applyServerAcls(lists: PolicyList[], roomIds: string[]): Promise { const serverName: string = new UserID(await this.client.getUserId()).domain; // Construct a server ACL first @@ -315,7 +316,7 @@ export class ProtectedRoomsSet { await this.client.sendNotice(this.managementRoomId, `Constructed server ACL:\n${JSON.stringify(finalAcl, null, 2)}`); } - const errors: RoomUpdateError[] = []; + const errors: IRoomUpdateError[] = []; for (const roomId of roomIds) { try { await this.managementRoomOutput.logMessage(LogLevel.DEBUG, "ApplyAcl", `Checking ACLs for ${roomId}`, roomId); @@ -340,8 +341,8 @@ export class ProtectedRoomsSet { } } catch (e) { const message = e.message || (e.body ? e.body.error : ''); - const kind = message && message.includes("You don't have permission to post that to the room") ? ERROR_KIND_PERMISSION : ERROR_KIND_FATAL; - errors.push({ roomId, errorMessage: message, errorKind: kind }); + const kind = message && message.includes("You don't have permission to post that to the room") ? CommandExceptionKind.Known : CommandExceptionKind.Unknown; + errors.push(new RoomUpdateException(roomId, kind, e, message)) } } return errors; @@ -353,17 +354,18 @@ export class ProtectedRoomsSet { * @param {string[]} roomIds The room IDs to apply the bans in. * @param {Mjolnir} mjolnir The Mjolnir client to apply the bans with. */ - private async applyUserBans(roomIds: string[]): Promise { + private async applyUserBans(roomIds: string[]): Promise { // We can only ban people who are not already banned, and who match the rules. - const errors: RoomUpdateError[] = []; + const errors: IRoomUpdateError[] = []; const addErrorToReport = (roomId: string, e: any) => { const message = e.message || (e.body ? e.body.error : ''); - errors.push({ + errors.push(new RoomUpdateException( roomId, - errorMessage: message, - errorKind: message && message.includes("You don't have permission to ban") ? ERROR_KIND_PERMISSION : ERROR_KIND_FATAL, - }); + message && message.includes("You don't have permission to ban") ? CommandExceptionKind.Known : CommandExceptionKind.Unknown, + e, + message + )); }; for (const roomId of roomIds) { @@ -461,28 +463,25 @@ export class ProtectedRoomsSet { } private async printActionResult( - errors: RoomUpdateError[], + errors: IRoomUpdateError[], renderOptions: { title?: string, noErrorsText?: string } ): Promise { printActionResult(this.client, this.managementRoomId, errors, renderOptions); } - public async unbanUser(user: string): Promise { - const errors: RoomUpdateError[] = []; + public async unbanUser(user: string): Promise { + const errors: IRoomUpdateError[] = []; for (const room of this.protectedRoomActivityTracker.protectedRoomsByActivity()) { try { await this.client.unbanUser(user, room); } catch (e) { - // FIXME: We need to know if `info` is acceptable or not. - // Technically these kinds of errors are expected to occur, - // so not sure if it warrants reporting as ERROR. - LogService.info('ProtectedRoomSet', `Unable to unban a user ${user} from ${room}`, e); const message = e.message || (e.body ? e.body.error : ''); - errors.push({ - roomId: room, - errorMessage: message, - errorKind: message && message.includes("You don't have permission to ban") ? ERROR_KIND_PERMISSION : ERROR_KIND_FATAL, - }); + errors.push(new RoomUpdateException( + room, + message && message.includes("You don't have permission to ban") ? CommandExceptionKind.Known : CommandExceptionKind.Unknown, + e, + message + )); } } return errors; @@ -493,7 +492,7 @@ export class ProtectedRoomsSet { } public async verifyPermissions() { - const errors: RoomUpdateError[] = []; + const errors: IRoomUpdateError[] = []; for (const roomId of this.protectedRooms) { errors.push(...(await this.protectionManager.verifyPermissionsIn(roomId))); } diff --git a/src/models/RoomUpdateError.tsx b/src/models/RoomUpdateError.tsx index 01388eb5..c98e2ba7 100644 --- a/src/models/RoomUpdateError.tsx +++ b/src/models/RoomUpdateError.tsx @@ -26,24 +26,48 @@ limitations under the License. */ import { UserID } from "matrix-bot-sdk"; +import { CommandException, CommandExceptionKind } from "../commands/interface-manager/CommandException"; import { DocumentNode } from "../commands/interface-manager/DeadDocument"; import { renderMatrixAndSend } from "../commands/interface-manager/DeadDocumentMatrix"; import { JSXFactory } from "../commands/interface-manager/JSXFactory"; import { Permalinks } from "../commands/interface-manager/Permalinks"; +import { CommandError, CommandResult } from "../commands/interface-manager/Validation"; import { MatrixSendClient } from "../MatrixEmitter"; -export const ERROR_KIND_PERMISSION = "permission"; -export const ERROR_KIND_FATAL = "fatal"; - -export interface RoomUpdateError { - roomId: string; - errorMessage: string; - errorKind: string; +export interface IRoomUpdateError extends CommandError { + readonly roomId: string, } -function renderErrorItem(error: RoomUpdateError, viaServers: string[]): DocumentNode { +export class PermissionError extends CommandError implements IRoomUpdateError { + constructor( + public readonly roomId: string, + message: string + ) { + super(message); + } +} + +export class RoomUpdateException extends CommandException implements IRoomUpdateError { + roomId: string; + + constructor(public readonly: string, ...args: ConstructorParameters) { + super(...args); + } + + public static Result( + message: string, + options: { + exception: Error, + exceptionKind: CommandExceptionKind, + roomId: string + }): CommandResult { + return CommandResult.Err(new RoomUpdateException(options.roomId, options.exceptionKind, options.exception, message)); + } +} + +function renderErrorItem(error: IRoomUpdateError, viaServers: string[]): DocumentNode { return
  • - {error.roomId} - {error.errorMessage} + {error.roomId} - {error.message}
    • } @@ -58,7 +82,7 @@ function renderErrorItem(error: RoomUpdateError, viaServers: string[]): Document */ export async function renderActionResult( client: MatrixSendClient, - errors: RoomUpdateError[], + errors: IRoomUpdateError[], { title = 'There were errors updating protected rooms.', noErrorsText = 'Done updating rooms - no errors.'}: { title?: string, noErrorsText?: string } = {} ): Promise { if (errors.length === 0) { @@ -97,7 +121,7 @@ export async function renderActionResult( export async function printActionResult( client: MatrixSendClient, roomId: string, - errors: RoomUpdateError[], + errors: IRoomUpdateError[], renderOptions: { title?: string, noErrorsText?: string } = {} ): Promise { await renderMatrixAndSend( diff --git a/src/protections/BanPropagation.tsx b/src/protections/BanPropagation.tsx index 46fb4d05..432387a9 100644 --- a/src/protections/BanPropagation.tsx +++ b/src/protections/BanPropagation.tsx @@ -40,7 +40,8 @@ import { MatrixRoomReference } from "../commands/interface-manager/MatrixRoomRef import { findPolicyListFromRoomReference } from "../commands/Ban"; import PolicyList from "../models/PolicyList"; import { renderListRules } from "../commands/Rules"; -import { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION, printActionResult, RoomUpdateError } from "../models/RoomUpdateError"; +import { printActionResult, IRoomUpdateError, RoomUpdateException } from "../models/RoomUpdateError"; +import { CommandExceptionKind } from "../commands/interface-manager/CommandException"; const BAN_PROPAGATION_PROMPT_LISTENER = 'ge.applied-langua.ge.draupnir.ban_propagation'; const UNBAN_PROPAGATION_PROMPT_LISTENER = 'ge.applied-langua.ge.draupnir.unban_propagation'; @@ -157,19 +158,20 @@ async function banReactionListener(this: ListenerContext, key: string, item: unk } } -async function unbanFromAllLists(mjolnir: Mjolnir, user: string): Promise { - const errors: RoomUpdateError[] = []; +async function unbanFromAllLists(mjolnir: Mjolnir, user: string): Promise { + const errors: IRoomUpdateError[] = []; for (const list of mjolnir.policyListManager.lists) { try { await list.unbanEntity(RULE_USER, user); } catch (e) { LogService.info('BanPropagation', `Could not unban ${user} from ${list.roomRef}`, e); const message = e.message || (e.body ? e.body.error : ''); - errors.push({ - roomId: list.roomId, - errorMessage: message, - errorKind: message.includes("You don't have permission") ? ERROR_KIND_PERMISSION : ERROR_KIND_FATAL - }) + errors.push(new RoomUpdateException( + list.roomId, + message.includes("You don't have permission") ? CommandExceptionKind.Known : CommandExceptionKind.Unknown, + e, + message + )); } } return errors; diff --git a/src/protections/ProtectionManager.ts b/src/protections/ProtectionManager.ts index c422f8a1..66ef815d 100644 --- a/src/protections/ProtectionManager.ts +++ b/src/protections/ProtectionManager.ts @@ -39,11 +39,11 @@ import { LogLevel, LogService } from "matrix-bot-sdk"; import { ProtectionSettingValidationError } from "./ProtectionSettings"; import { Consequence } from "./consequence"; import { htmlEscape } from "../utils"; -import { ERROR_KIND_FATAL, ERROR_KIND_PERMISSION } from "../models/RoomUpdateError"; -import { RoomUpdateError } from "../models/RoomUpdateError"; +import { IRoomUpdateError, PermissionError, RoomUpdateException } from "../models/RoomUpdateError"; import { BanPropagation } from "./BanPropagation"; import { MatrixDataManager, RawSchemedData, SchemaMigration, SCHEMA_VERSION_KEY } from "../models/MatrixDataManager"; import { Permalinks } from "../commands/interface-manager/Permalinks"; +import { CommandExceptionKind } from "../commands/interface-manager/CommandException"; const PROTECTIONS: Protection[] = [ new FirstMessageIsImage(), @@ -384,8 +384,8 @@ export class ProtectionManager { return new Set(this.enabledProtections.map((p) => p.requiredStatePermissions).flat()) } - public async verifyPermissionsIn(roomId: string): Promise { - const errors: RoomUpdateError[] = []; + public async verifyPermissionsIn(roomId: string): Promise { + const errors: IRoomUpdateError[] = []; const additionalPermissions = this.requiredProtectionPermissions(); try { @@ -413,35 +413,21 @@ export class ProtectionManager { const userLevel = plDefault(users[ownUserId], usersDefault); const aclLevel = plDefault(events["m.room.server_acl"], stateDefault); - // Wants: ban, kick, redact, m.room.server_acl + const addErrorToReport = (message: string) => { + errors.push(new PermissionError(roomId, message)) + } if (userLevel < ban) { - errors.push({ - roomId, - errorMessage: `Missing power level for bans: ${userLevel} < ${ban}`, - errorKind: ERROR_KIND_PERMISSION, - }); + addErrorToReport(`Missing power level for bans: ${userLevel} < ${ban}`); } if (userLevel < kick) { - errors.push({ - roomId, - errorMessage: `Missing power level for kicks: ${userLevel} < ${kick}`, - errorKind: ERROR_KIND_PERMISSION, - }); + addErrorToReport(`Missing power level for kicks: ${userLevel} < ${kick}`); } if (userLevel < redact) { - errors.push({ - roomId, - errorMessage: `Missing power level for redactions: ${userLevel} < ${redact}`, - errorKind: ERROR_KIND_PERMISSION, - }); + addErrorToReport(`Missing power level for redactions: ${userLevel} < ${redact}`); } if (!this.mjolnir.config.disableServerACL && userLevel < aclLevel) { - errors.push({ - roomId, - errorMessage: `Missing power level for server ACLs: ${userLevel} < ${aclLevel}`, - errorKind: ERROR_KIND_PERMISSION, - }); + addErrorToReport(`Missing power level for server ACLs: ${userLevel} < ${aclLevel}`); } // Wants: Additional permissions @@ -450,24 +436,15 @@ export class ProtectionManager { const permLevel = plDefault(events[additionalPermission], stateDefault); if (userLevel < permLevel) { - errors.push({ - roomId, - errorMessage: `Missing power level for "${additionalPermission}" state events: ${userLevel} < ${permLevel}`, - errorKind: ERROR_KIND_PERMISSION, - }); + addErrorToReport(`Missing power level for "${additionalPermission}" state events: ${userLevel} < ${permLevel}`); } } // Otherwise OK } catch (e) { - LogService.error("Mjolnir", e); - errors.push({ - roomId, - errorMessage: e.message || (e.body ? e.body.error : ''), - errorKind: ERROR_KIND_FATAL, - }); + const message = `Unexpected error when attempting to verify the permissions in ${roomId}`; + errors.push(new RoomUpdateException(roomId, CommandExceptionKind.Unknown, e, message)); } - return errors; } diff --git a/src/queues/EventRedactionQueue.ts b/src/queues/EventRedactionQueue.ts index 2a6d6f3b..8806921f 100644 --- a/src/queues/EventRedactionQueue.ts +++ b/src/queues/EventRedactionQueue.ts @@ -25,11 +25,11 @@ limitations under the License. * are NOT distributed, contributed, committed, or licensed under the Apache License. */ import { LogLevel, MatrixClient } from "matrix-bot-sdk" -import { ERROR_KIND_FATAL } from "../models/RoomUpdateError"; -import { RoomUpdateError } from "../models/RoomUpdateError"; +import { IRoomUpdateError, RoomUpdateException } from "../models/RoomUpdateError"; import { redactUserMessagesIn } from "../utils"; import ManagementRoomOutput from "../ManagementRoomOutput"; import { MatrixSendClient } from "../MatrixEmitter"; +import { CommandExceptionKind } from "../commands/interface-manager/CommandException"; export interface QueuedRedaction { /** The room which the redaction will take place in. */ @@ -119,25 +119,21 @@ export class EventRedactionQueue { * @param limitToRoomId If the roomId is provided, only redactions for that room will be processed. * @returns A description of any errors encountered by each QueuedRedaction that was processed. */ - public async process(client: MatrixSendClient, managementRoom: ManagementRoomOutput, limitToRoomId?: string): Promise { - const errors: RoomUpdateError[] = []; + public async process(client: MatrixSendClient, managementRoom: ManagementRoomOutput, limitToRoomId?: string): Promise { + const errors: IRoomUpdateError[] = []; const redact = async (currentBatch: QueuedRedaction[]) => { for (const redaction of currentBatch) { try { await redaction.redact(client, managementRoom); } catch (e) { - let roomError: RoomUpdateError; - if (e.roomId && e.errorMessage && e.errorKind) { - roomError = e; - } else { - const message = e.message || (e.body ? e.body.error : ''); - roomError = { - roomId: redaction.roomId, - errorMessage: message, - errorKind: ERROR_KIND_FATAL, - }; - } - errors.push(roomError); + const message = e.message || (e.body ? e.body.error : ''); + const error = new RoomUpdateException( + redaction.roomId, + CommandExceptionKind.Unknown, + e, + message + ); + errors.push(error); } } }