diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index 602f0dc..417e532 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -42,7 +42,7 @@ import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQu import * as htmlEscape from "escape-html"; import { ReportManager } from "./report/ReportManager"; import { WebAPIs } from "./webapis/WebAPIs"; -import { makeClientWithSanerExceptions } from "./utils"; +import { isClientWithSanerExceptions } from "./utils"; export const STATE_NOT_STARTED = "not_started"; export const STATE_CHECKING_PERMISSIONS = "checking_permissions"; @@ -55,7 +55,6 @@ const PROTECTED_ROOMS_EVENT_TYPE = "org.matrix.mjolnir.protected_rooms"; const WARN_UNPROTECTED_ROOM_EVENT_PREFIX = "org.matrix.mjolnir.unprotected_room_warning.for."; export class Mjolnir { - public readonly client: MatrixClient; private displayName: string; private localpart: string; private currentState: string = STATE_NOT_STARTED; @@ -154,12 +153,15 @@ export class Mjolnir { } constructor( - client: MatrixClient, + public readonly client: MatrixClient, public readonly protectedRooms: { [roomId: string]: string }, private banLists: BanList[], ) { + if (!isClientWithSanerExceptions(client)) { + throw new Error("Internal error: this is a MatrixClient without sane exceptions"); + } + this.explicitlyProtectedRoomIds = Object.keys(this.protectedRooms); - this.client = makeClientWithSanerExceptions(client); for (const reason of config.automaticallyRedactForReasons) { this.automaticRedactionReasons.push(new MatrixGlob(reason.toLowerCase())); diff --git a/src/index.ts b/src/index.ts index 11550b8..58325d4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,6 +27,7 @@ import config from "./config"; import { logMessage } from "./LogProxy"; import { Healthz } from "./health/healthz"; import { Mjolnir } from "./Mjolnir"; +import { makeClientWithSanerExceptions } from "./utils"; config.RUNTIME = {}; @@ -51,6 +52,7 @@ if (config.health.healthz.enabled) { } else { client = new MatrixClient(config.homeserverUrl, config.accessToken, storage); } + client = makeClientWithSanerExceptions(client); config.RUNTIME.client = client; diff --git a/src/utils.ts b/src/utils.ts index ca227d2..ea485f5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -203,54 +203,100 @@ export async function replaceRoomIdsWithPills(client: MatrixClient, text: string return content; } +/** + * Utility function: a wrapper for `MatrixClient.doRequest` that makes sure that + * we never throw an `IncomingMessage`. + * + * @param client The client to use for the request. + * @param args The args to pass to the client. + * @returns As `client.doRequest(...args)` but with `IncomingMessage` errors wrapped + * as instances of `Error`. + */ +async function doRequestReplacement(client: MatrixClient, args: any[]): Promise { + try { + return await client.doRequest.apply(client, args); + } catch (ex) { + if (!(ex instanceof IncomingMessage)) { + // In most cases, we're happy with the result. + throw ex; + } + // However, MatrixClient has a tendency of throwing + // instances of `IncomingMessage` instead of instances + // of `Error`. The former take ~800 lines of log and + // provide no stack trace, which makes them typically + // useless. + let method: string | null = null; + let path = ''; + let body: string | null = null; + if (ex.method) { + method = ex.method; + } + if (ex.url) { + path = ex.url; + } + if ("req" in ex && (ex as any).req instanceof ClientRequest) { + if (!method) { + method = (ex as any).req.method; + } + if (!path) { + path = (ex as any).req.path; + } + } + if ("body" in ex) { + body = JSON.stringify((ex as any).body); + } + let error = new Error(`Error during MatrixClient request ${method} ${path}: ${ex.statusCode} ${ex.statusMessage} -- ${body}`); + throw error; + } +} + + +// A key used internally to determine whether a `MatrixClient` has been monkey patched +// to return saner exceptions. +const CLIENT_WITH_SANER_EXCEPTION = Symbol("_monkeyPatchmakeClientWithSanerExceptions"); + +/** + * @returns `true` if a `MatrixClient` has been monkey patched to return + * saner exceptions. + */ +export function isClientWithSanerExceptions(client: MatrixClient): boolean { + return CLIENT_WITH_SANER_EXCEPTION in client; +} + +/** + * Wrap a `MatrixClient` into something that throws sane exceptions. + * + * By default, instances of `MatrixClient` throw instances of `IncomingMessage` + * in case of many errors. Unfortunately, these instances are unusable: + * + * - they are logged as ~800 *lines of code*; + * - there is no error message; + * - they offer no stack. + * + * This method converts a `MatrixClient` that may throw `IncomingMessage` into + * a `MatrixClient` that instead throws more reasonable insetances of `Error`. + */ export function makeClientWithSanerExceptions(client: MatrixClient): MatrixClient { let result = new Proxy(client, { + has: function (obj, key): boolean { + return key === CLIENT_WITH_SANER_EXCEPTION + || key in client; + }, get: function (obj, key) { + if (key === "doRequest") { + // Intercept `doRequest`. + return (...args) => doRequestReplacement(client, args); + } + if (key === "_monkeyPatchmakeClientWithSanerExceptions") { + return true; + } let value = obj[key]; - if (!(typeof value == "function")) { + if (!(typeof value === "function")) { + // We're only interested in methods. return value; } - return function (...args) { - let result = value.apply(client, args); - if (!(result instanceof Promise)) { - // We're only interested in watching async code. - return result; - } - return result.catch(reason => { - if (!(reason instanceof IncomingMessage)) { - // In most cases, we're happy with the result. - throw reason; - } - // However, MatrixClient has a tendency of throwing - // instances of `IncomingMessage` instead of instances - // of `Error`. The former take ~800 lines of log and - // provide no stack trace, which makes them typically - // useless. - let method: string | null = null; - let path: string = ''; - let body: string | null = null; - if (reason.method) { - method = reason.method; - } - if (reason.url) { - path = reason.url; - } - if ("req" in reason && (reason as any).req instanceof ClientRequest) { - if (!method) { - method = (reason as any).req.method; - } - if (!path) { - path = (reason as any).req.path; - } - } - if ("body" in reason) { - body = JSON.stringify((reason as any).body); - } - let error = new Error(`Error during MatrixClient request ${method} ${path}: ${reason.statusCode} ${reason.statusMessage} -- ${body}`); - //(error as any).message = reason; - throw error; - }); - } + // Make sure that methods use our intercepted `doRequestReplacement`. + return value.bind(result); } }); return result; diff --git a/src/webapis/WebAPIs.ts b/src/webapis/WebAPIs.ts index 9d7487a..ef0375b 100644 --- a/src/webapis/WebAPIs.ts +++ b/src/webapis/WebAPIs.ts @@ -21,6 +21,7 @@ import { MatrixClient } from "matrix-bot-sdk"; import config from "../config"; import { ReportManager } from "../report/ReportManager"; +import { makeClientWithSanerExceptions } from "../utils"; /** * A common prefix for all web-exposed APIs. @@ -130,7 +131,7 @@ export class WebAPIs { // so we are not extending the abilities of Mjölnir // 3. We are avoiding the use of the Synapse Admin API to ensure that // this feature can work with all homeservers, not just Synapse. - let reporterClient = new MatrixClient(config.rawHomeserverUrl, accessToken); + let reporterClient = makeClientWithSanerExceptions(new MatrixClient(config.rawHomeserverUrl, accessToken)); reporterClient.start = () => { throw new Error("We MUST NEVER call start on the reporter client"); }; diff --git a/test/integration/clientHelper.ts b/test/integration/clientHelper.ts index 169a1ee..b478c73 100644 --- a/test/integration/clientHelper.ts +++ b/test/integration/clientHelper.ts @@ -1,6 +1,6 @@ import axios from "axios"; import { HmacSHA1 } from "crypto-js"; -import { LogService, MatrixClient, MemoryStorageProvider, PantalaimonClient } from "matrix-bot-sdk"; +import { extractRequestError, LogService, MatrixClient, MemoryStorageProvider, PantalaimonClient } from "matrix-bot-sdk"; import config from "../../src/config"; import { makeClientWithSanerExceptions } from "../../src/utils"; @@ -27,7 +27,7 @@ export async function registerUser(username: string, displayname: string, passwo password, admin, mac: mac.toString() - }) + }); } /** diff --git a/test/integration/mjolnirSetupUtils.ts b/test/integration/mjolnirSetupUtils.ts index 3e6dd49..d1165d0 100644 --- a/test/integration/mjolnirSetupUtils.ts +++ b/test/integration/mjolnirSetupUtils.ts @@ -24,6 +24,7 @@ import { import { Mjolnir} from '../../src/Mjolnir'; import config from "../../src/config"; import { registerUser } from "./clientHelper"; +import { makeClientWithSanerExceptions } from "../../src/utils"; /** * Ensures that a room exists with the alias, if it does not exist we create it. @@ -79,7 +80,8 @@ export async function makeMjolnir(): Promise { LogService.setLevel(LogLevel.fromString(config.logLevel, LogLevel.DEBUG)); LogService.info("test/mjolnirSetupUtils", "Starting bot..."); const pantalaimon = new PantalaimonClient(config.homeserverUrl, new MemoryStorageProvider()); - const client = await pantalaimon.createClientWithCredentials(config.pantalaimon.username, config.pantalaimon.password); + let client = await pantalaimon.createClientWithCredentials(config.pantalaimon.username, config.pantalaimon.password); + client = makeClientWithSanerExceptions(client); await ensureAliasedRoomExists(client, config.managementRoom); let mjolnir = await Mjolnir.setupMjolnirFromConfig(client); globalClient = client;