From eece53024fbe5933317c88f185ad8f6a964442d3 Mon Sep 17 00:00:00 2001
From: "Kai A. Hiller" <V02460@gmail.com>
Date: Mon, 24 Jun 2019 11:25:00 -0400
Subject: [PATCH] Refactor error handling

Signed-off-by: Kai A. Hiller <V02460@gmail.com>
---
 package.json                      |  2 ++
 src/matrixeventprocessor.ts       | 43 +++++++++++++++++++++++--------
 src/util.ts                       | 22 ++++++++++++++++
 test/test_matrixeventprocessor.ts | 13 ++++++----
 4 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/package.json b/package.json
index 60894a2..f7b0259 100644
--- a/package.json
+++ b/package.json
@@ -34,7 +34,9 @@
   },
   "homepage": "https://github.com/Half-Shot/matrix-appservice-discord#readme",
   "dependencies": {
+    "@types/chai-as-promised": "^7.1.0",
     "better-sqlite3": "^5.0.1",
+    "chai-as-promised": "^7.1.1",
     "command-line-args": "^4.0.1",
     "command-line-usage": "^4.1.0",
     "discord-markdown": "^2.0.0",
diff --git a/src/matrixeventprocessor.ts b/src/matrixeventprocessor.ts
index 5751c8d..e286cc1 100644
--- a/src/matrixeventprocessor.ts
+++ b/src/matrixeventprocessor.ts
@@ -18,10 +18,19 @@ import * as Discord from "discord.js";
 import { DiscordBot } from "./bot";
 import { DiscordBridgeConfig } from "./config";
 import * as escapeStringRegexp from "escape-string-regexp";
-import { Util } from "./util";
+import { Util, wrap } from "./util";
 import * as path from "path";
 import * as mime from "mime";
-import { MatrixUser, Bridge, BridgeContext } from "matrix-appservice-bridge";
+import {
+    Bridge,
+    BridgeContext,
+    EventInternalError,
+    EventNotHandledError,
+    EventTooOldError,
+    EventUnknownError,
+    MatrixUser,
+    Request,
+} from "matrix-appservice-bridge";
 import { Client as MatrixClient } from "matrix-js-sdk";
 import { IMatrixEvent, IMatrixEventContent, IMatrixMessage } from "./matrixtypes";
 import { MatrixMessageProcessor, IMatrixMessageProcessorParams } from "./matrixmessageprocessor";
@@ -72,11 +81,17 @@ export class MatrixEventProcessor {
         }
     }
 
-    public async OnEvent(request, context: BridgeContext): Promise<void> {
+    /**
+     * Callback which is called when the HS notifies the bridge of a new event.
+     *
+     * @param request Request object containing the event for which this callback is called.
+     * @param context The current context of the bridge.
+     * @throws {EventNotHandledError} When the event can finally not be handled.
+     */
+    public async OnEvent(request: Request, context: BridgeContext): Promise<void> {
         const event = request.getData() as IMatrixEvent;
         if (event.unsigned.age > AGE_LIMIT) {
-            log.warn(`Skipping event due to age ${event.unsigned.age} > ${AGE_LIMIT}`);
-            return;
+            throw new EventTooOldError(`Skipping event due to age ${event.unsigned.age} > ${AGE_LIMIT}`);
         }
         if (
             event.type === "m.room.member" &&
@@ -106,7 +121,11 @@ export class MatrixEventProcessor {
         } else if (["m.room.member", "m.room.name", "m.room.topic"].includes(event.type)) {
             await this.ProcessStateEvent(event);
             return;
-        } else if (event.type === "m.room.redaction" && context.rooms.remote) {
+        } else if (event.type === "m.room.redaction") {
+            if (!context.rooms.remote) {
+                log.info("Got radaction event with no linked room. Ignoring.");
+                return;
+            }
             await this.discord.ProcessMatrixRedact(event);
             return;
         } else if (event.type === "m.room.message" || event.type === "m.sticker") {
@@ -123,21 +142,23 @@ export class MatrixEventProcessor {
                     await this.ProcessMsgEvent(event, srvChanPair[0], srvChanPair[1]);
                     return;
                 } catch (err) {
-                    log.warn("There was an error sending a matrix event", err);
-                    return;
+                    throw wrap(err, Error, "There was an error sending a matrix event");
                 }
+            } else {
+                log.info("Got event with no linked room. Ignoring.");
+                return;
             }
         } else if (event.type === "m.room.encryption" && context.rooms.remote) {
             try {
                 await this.HandleEncryptionWarning(event.room_id);
                 return;
             } catch (err) {
-                throw new Error(`Failed to handle encrypted room, ${err}`);
+                throw wrap(err, EventNotHandledError, `Failed to handle encrypted room, ${err}`);
             }
         } else {
-            log.verbose("Got non m.room.message event");
+            throw new EventUnknownError("Got non m.room.message event");
         }
-        log.verbose("Event not processed by bridge");
+        throw new EventUnknownError(); // Shouldn't be reachable
     }
 
     public async HandleEncryptionWarning(roomId: string): Promise<void> {
diff --git a/src/util.ts b/src/util.ts
index e016fcd..f8da79e 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -431,3 +431,25 @@ type Type = Function;  // tslint:disable-line ban-types
 export function instanceofsome(obj: object, types: Type[]): boolean {
     return types.some((type) => obj instanceof type);
 }
+
+/**
+ * Append the old error message to the new one and keep its stack trace.
+ * Example:
+ *     throw wrap(e, HighLevelError, "This error is more specific");
+ */
+export function wrap<T extends Error>(
+    oldError: Type,
+    newErrorType: new (...args: any[]) => T,  // tslint:disable-line no-any
+    ...args: any[]  // tslint:disable-line no-any trailing-comma
+): T {
+    const newError = new newErrorType(...args);
+    let appendMsg;
+    if (oldError instanceof Error) {
+        appendMsg = oldError.message;
+        newError.stack = oldError.stack;
+    } else {
+        appendMsg = oldError.toString();
+    }
+    newError.message += ":\n" + appendMsg;
+    return newError;
+}
diff --git a/test/test_matrixeventprocessor.ts b/test/test_matrixeventprocessor.ts
index e5dbf0c..033ce45 100644
--- a/test/test_matrixeventprocessor.ts
+++ b/test/test_matrixeventprocessor.ts
@@ -15,8 +15,10 @@ limitations under the License.
 */
 
 import * as Chai from "chai";
+import * as ChaiAsPromised from "chai-as-promised";
 import * as Discord from "discord.js";
 import * as Proxyquire from "proxyquire";
+import { EventTooOldError, EventUnknownError} from "matrix-appservice-bridge";
 
 import { PresenceHandler } from "../src/presencehandler";
 import { DiscordBot } from "../src/bot";
@@ -34,6 +36,7 @@ import { IMatrixEvent } from "../src/matrixtypes";
 
 const TEST_TIMESTAMP = 1337;
 
+Chai.use(ChaiAsPromised);
 const expect = Chai.expect;
 // const assert = Chai.assert;
 function buildRequest(eventData) {
@@ -811,20 +814,20 @@ This is the reply`,
     });
     describe("OnEvent", () => {
         it("should reject old events", async () => {
-            const AGE = 900001; // 15 * 60 * 1000
+            const AGE = 900001; // 15 * 60 * 1000 + 1
             const processor = createMatrixEventProcessor();
-            await processor.OnEvent(buildRequest({unsigned: {age: AGE}}), null);
-            expect(MESSAGE_PROCCESS).equals("");
+            const callbackPromise = processor.OnEvent(buildRequest({unsigned: {age: AGE}}), null);
+            expect(callbackPromise).to.eventually.be.rejectedWith(EventTooOldError);
         });
         it("should reject un-processable events", async () => {
             const AGE = 900000; // 15 * 60 * 1000
             const processor = createMatrixEventProcessor();
             // check if nothing is thrown
-            await processor.OnEvent(buildRequest({
+            const callbackPromise =  processor.OnEvent(buildRequest({
                 content: {},
                 type: "m.potato",
                 unsigned: {age: AGE}}), null);
-            expect(MESSAGE_PROCCESS).equals("");
+            expect(callbackPromise).to.eventually.be.rejectedWith(EventUnknownError);
         });
         it("should handle own invites", async () => {
             const processor = createMatrixEventProcessor();
-- 
GitLab