diff --git a/package.json b/package.json index 25c4e12..70ebb17 100644 --- a/package.json +++ b/package.json @@ -10,9 +10,9 @@ "scripts": { "build": "tsc", "lint": "tslint --project ./tsconfig.json -t stylish", - "start:dev": "yarn build && node lib/index.js", + "start:dev": "yarn build && node --async-stack-traces lib/index.js", "test": "ts-mocha --project ./tsconfig.json test/commands/**/*.ts", - "test:integration": "NODE_ENV=harness ts-mocha --require test/integration/fixtures.ts --project ./tsconfig.json \"test/integration/**/*Test.ts\"", + "test:integration": "NODE_ENV=harness ts-mocha --async-stack-traces --require test/integration/fixtures.ts --project ./tsconfig.json \"test/integration/**/*Test.ts\"", "test:manual": "NODE_ENV=harness ts-node test/integration/manualLaunchScript.ts" }, "devDependencies": { diff --git a/src/index.ts b/src/index.ts index 1def8aa..1434225 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,7 +27,7 @@ import config from "./config"; import { logMessage } from "./LogProxy"; import { Healthz } from "./health/healthz"; import { Mjolnir } from "./Mjolnir"; -import { patchMatrixClientForConciseExceptions } from "./utils"; +import { patchMatrixClient } from "./utils"; config.RUNTIME = {}; @@ -52,7 +52,7 @@ if (config.health.healthz.enabled) { } else { client = new MatrixClient(config.homeserverUrl, config.accessToken, storage); } - patchMatrixClientForConciseExceptions(); + patchMatrixClient(); config.RUNTIME.client = client; let bot = await Mjolnir.setupMjolnirFromConfig(client); diff --git a/src/report/ReportManager.ts b/src/report/ReportManager.ts index 0d01f06..7b7b561 100644 --- a/src/report/ReportManager.ts +++ b/src/report/ReportManager.ts @@ -22,7 +22,6 @@ import { JSDOM } from 'jsdom'; import config from "../config"; import { Mjolnir } from "../Mjolnir"; -import { patchMatrixClientForConciseExceptions } from "../utils"; /// Regexp, used to extract the action label from an action reaction /// such as `⚽ Kick user @foobar:localhost from room [kick-user]`. @@ -876,7 +875,6 @@ class DisplayManager { if (!await action.canExecute(this.owner, report, moderationRoomId)) { continue; } - patchMatrixClientForConciseExceptions(); await this.owner.mjolnir.client.sendEvent(config.managementRoom, "m.reaction", { "m.relates_to": { "rel_type": "m.annotation", diff --git a/src/utils.ts b/src/utils.ts index 644331b..df76d1c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -220,14 +220,15 @@ let isMatrixClientPatchedForConciseExceptions = false; * This method configures `MatrixClient` to ensure that methods that may throw * instead throws more reasonable insetances of `Error`. */ -export function patchMatrixClientForConciseExceptions() { +function patchMatrixClientForConciseExceptions() { if (isMatrixClientPatchedForConciseExceptions) { return; } let originalRequestFn = getRequestFn(); setRequestFn((params, cb) => { + let stack = new Error().stack; originalRequestFn(params, function conciseExceptionRequestFn(err, response, resBody) { - if (!err && (response.statusCode < 200 || response.statusCode >= 300)) { + if (!err && (response?.statusCode < 200 || response?.statusCode >= 300)) { // Normally, converting HTTP Errors into rejections is done by the caller // of `requestFn` within matrix-bot-sdk. However, this always ends up rejecting // with an `IncomingMessage` - exactly what we wish to avoid here. @@ -277,12 +278,47 @@ export function patchMatrixClientForConciseExceptions() { } } if ("body" in err) { - body = JSON.stringify((err as any).body); + body = (err as any).body; } let error = new Error(`Error during MatrixClient request ${method} ${path}: ${err.statusCode} ${err.statusMessage} -- ${body}`); + error.stack = stack; + if (body) { + // Calling code may use `body` to check for errors, so let's + // make sure that we're providing it. + try { + body = JSON.parse(body); + } catch (ex) { + // Not JSON. + } + // Define the property but don't make it visible during logging. + Object.defineProperty(error, "body", { + value: body, + enumerable: false, + }); + } + // Calling code may use `statusCode` to check for errors, so let's + // make sure that we're providing it. + if ("statusCode" in err) { + Object.defineProperty(error, "statusCode", { + value: err.statusCode, + enumerable: false, + }); + } return cb(error, response, resBody); }) }); isMatrixClientPatchedForConciseExceptions = true; } +/** + * Perform any patching deemed necessary to MatrixClient. + */ +export function patchMatrixClient() { + // Note that the order of patches is meaningful. + // + // - `patchMatrixClientForConciseExceptions` converts all `IncomingMessage` + // errors into instances of `Error` handled as errors; + // - `patchMatrixClientForRetry` expects that all errors are returned as + // errors. + patchMatrixClientForConciseExceptions(); +} diff --git a/test/integration/abuseReportTest.ts b/test/integration/abuseReportTest.ts index 6839b1f..e79d332 100644 --- a/test/integration/abuseReportTest.ts +++ b/test/integration/abuseReportTest.ts @@ -21,7 +21,7 @@ const REPORT_NOTICE_REGEXPS = { describe("Test: Reporting abuse", async () => { it('Mjölnir intercepts abuse reports', async function() { - this.timeout(10000); + this.timeout(60000); // Listen for any notices that show up. let notices = []; @@ -216,7 +216,7 @@ describe("Test: Reporting abuse", async () => { } }); it('The redact action works', async function() { - this.timeout(10000); + this.timeout(60000); // Listen for any notices that show up. let notices = []; diff --git a/test/integration/clientHelper.ts b/test/integration/clientHelper.ts index 45e8c14..6bb2bbc 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 { extractRequestError, LogService, MatrixClient, MemoryStorageProvider, PantalaimonClient } from "matrix-bot-sdk"; +import { LogService, MatrixClient, MemoryStorageProvider, PantalaimonClient } from "matrix-bot-sdk"; import config from "../../src/config"; /** diff --git a/test/integration/commands/redactCommandTest.ts b/test/integration/commands/redactCommandTest.ts index c7daaf3..5cb6320 100644 --- a/test/integration/commands/redactCommandTest.ts +++ b/test/integration/commands/redactCommandTest.ts @@ -8,7 +8,7 @@ import { onReactionTo } from "./commandUtils"; describe("Test: The redaction command", function () { it('Mjölnir redacts all of the events sent by a spammer when instructed to by giving their id and a room id.', async function() { - this.timeout(20000); + this.timeout(60000); // Create a few users and a room. let badUser = await newTestUser(false, "spammer-needs-redacting"); let badUserId = await badUser.getUserId(); @@ -52,7 +52,7 @@ import { onReactionTo } from "./commandUtils"; }); }) it('Mjölnir redacts all of the events sent by a spammer when instructed to by giving their id in multiple rooms.', async function() { - this.timeout(20000); + this.timeout(60000); // Create a few users and a room. let badUser = await newTestUser(false, "spammer-needs-redacting"); let badUserId = await badUser.getUserId(); @@ -101,7 +101,7 @@ import { onReactionTo } from "./commandUtils"; }); }); it("Redacts a single event when instructed to.", async function () { - this.timeout(20000); + this.timeout(60000); // Create a few users and a room. let badUser = await newTestUser(false, "spammer-needs-redacting"); const mjolnir = config.RUNTIME.client! diff --git a/test/integration/mjolnirSetupUtils.ts b/test/integration/mjolnirSetupUtils.ts index 13a0d75..b30b3ae 100644 --- a/test/integration/mjolnirSetupUtils.ts +++ b/test/integration/mjolnirSetupUtils.ts @@ -24,7 +24,7 @@ import { import { Mjolnir} from '../../src/Mjolnir'; import config from "../../src/config"; import { registerUser } from "./clientHelper"; -import { patchMatrixClientForConciseExceptions } from "../../src/utils"; +import { patchMatrixClient } from "../../src/utils"; /** * Ensures that a room exists with the alias, if it does not exist we create it. @@ -33,8 +33,9 @@ import { patchMatrixClientForConciseExceptions } from "../../src/utils"; * @returns The room ID of the aliased room. */ export async function ensureAliasedRoomExists(client: MatrixClient, alias: string): Promise { - return await client.resolveRoom(alias) - .catch(async e => { + try { + return await client.resolveRoom(alias); + } catch (e) { if (e?.body?.errcode === 'M_NOT_FOUND') { console.info(`${alias} hasn't been created yet, so we're making it now.`) let roomId = await client.createRoom({ @@ -44,7 +45,7 @@ export async function ensureAliasedRoomExists(client: MatrixClient, alias: strin return roomId } throw e; - }); + } } async function configureMjolnir() { @@ -81,7 +82,7 @@ export async function makeMjolnir(): Promise { 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); - patchMatrixClientForConciseExceptions(); + patchMatrixClient(); await ensureAliasedRoomExists(client, config.managementRoom); let mjolnir = await Mjolnir.setupMjolnirFromConfig(client); globalClient = client; diff --git a/test/integration/timelinePaginationTest.ts b/test/integration/timelinePaginationTest.ts index 5e36458..d746982 100644 --- a/test/integration/timelinePaginationTest.ts +++ b/test/integration/timelinePaginationTest.ts @@ -8,7 +8,7 @@ import { getMessagesByUserIn } from "../../src/utils"; */ describe("Test: timeline pagination", function () { it('does not paginate across the entire room history while backfilling.', async function() { - this.timeout(20000); + this.timeout(60000); // Create a few users and a room. let badUser = await newTestUser(false, "spammer"); let badUserId = await badUser.getUserId(); @@ -38,7 +38,7 @@ describe("Test: timeline pagination", function () { assert.equal(eventCount, 7, "There shouldn't be any more events (1 member event and 6 messages), and they should all be from the same account."); }) it('does not call the callback with an empty array when there are no relevant events', async function() { - this.timeout(20000); + this.timeout(60000); let badUser = await newTestUser(false, "spammer"); let badUserId = await badUser.getUserId(); let moderator = await newTestUser(false, "moderator"); @@ -53,7 +53,7 @@ describe("Test: timeline pagination", function () { assert.equal(cbCount, 0, "The callback should never get called"); }) it("The limit provided is respected", async function() { - this.timeout(20000); + this.timeout(60000); let badUser = await newTestUser(false, "spammer"); let badUserId = await badUser.getUserId(); let moderator = await newTestUser(false, "moderator"); @@ -82,7 +82,7 @@ describe("Test: timeline pagination", function () { assert.equal(cbCount, 1, "The callback should be called once with events matching the glob."); }); it("Gives the events to the callback ordered by youngest first (even more important when the limit is reached halfway through a chunk).", async function() { - this.timeout(20000); + this.timeout(60000); let moderator = await newTestUser(false, "moderator"); let moderatorId = await moderator.getUserId(); let targetRoom = await moderator.createRoom();