Add support for a "v2" webhook transformation API (#223)

* Add support for a "v2" webhook transformation API

* changelog

changelog.d/223.feature

@@ -0,0 +1,2 @@
+Add support for `v2` webhook transformation functions, supporting more options.
+See https://matrix-org.github.io/matrix-hookshot/setup/webhooks.html#javascript-transformations for more information

docs/setup/webhooks.md

@@ -64,14 +64,50 @@ The script string should be set within the state event under the `transformation
 
 ### Script API
 
-The scripts have a very minimal API. The execution environment will contain a `data` field, which will be the body
-of the incoming request (JSON will be parsed into an `Object`). Scripts are executed syncronously and a variable `result`
-is expected to be set in the execution, which will be used as the text value for the script. `result` will be automatically
-transformed by a Markdown parser.
+Transformation scripts have a versioned API. You can check the version of the API that the hookshot instance supports
+at runtime by checking the `HookshotApiVersion` variable. If the variable is undefined, it should be considered `v1`.
 
-If the script contains errors or is otherwise unable to work, the bridge will send an error to the room.
+The execution environment will contain a `data` variable, which will be the body of the incoming request (JSON will be parsed into an `Object`).
+Scripts are executed syncronously and expect the `result` variable to be set.
 
-### Example script
+If the script contains errors or is otherwise unable to work, the bridge will send an error to the room. You can check the logs of the bridge
+for a more precise error.
+
+### V2 API
+
+The `v2` api expects an object to be returned from the `result` variable.
+
+```json5
+{
+  "version": "v2" // The version of the schema being returned from the function. This is always "v2".
+  "empty": true|false, // Should the webhook be ignored and no output returned. The default is false (plain must be provided).
+  "plain": "Some text", // The plaintext value to be used for the Matrix message.
+  "html": "<b>Some</b> text", // The HTML value to be used for the Matrix message. If not provided, plain will be interpreted as markdown.
+}
+```
+
+#### Example script
+
+Where `data` = `{"counter": 5, "maxValue": 4}`
+
+```js
+if (data.counter === undefined) {
+  // The API didn't give us a counter, send no message.
+  result = {empty: true, version: "v2"};
+} else if (data.counter > data.maxValue) {
+    result = {plain: `**Oh no!** The counter has gone over by ${data.counter - data.maxValue}`, version: "v2"};
+} else {
+    result = {plain: `*Everything is fine*, the counter is under by ${data.maxValue - data.counter}`, version: "v2"};
+}
+```
+
+
+### V1 API
+
+The v1 API expects `result` to be a string. The string will be automatically transformed into markdown. All webhook messages
+will be prefix'd with `Received webhook:`. If `result` is falsey (undefined, false or null) then the message will be `No content`.
+
+#### Example script
 
 Where `data` = `{"counter": 5, "maxValue": 4}`
 

src/Connections/GenericHook.ts

@@ -29,6 +29,13 @@ export interface GenericHookAccountData {
     [hookId: string]: string;
 }
 
+interface WebhookTransformationResult {
+    version: string;
+    plain?: string;
+    html?: string;
+    empty?: boolean;
+}
+
 const log = new LogWrapper("GenericHookConnection");
 const md = new markdownit();
 
@@ -195,13 +202,10 @@ export class GenericHookConnection extends BaseConnection implements IConnection
         return msg;
     }
 
-    public async onGenericHook(data: Record<string, unknown>) {
-        log.info(`onGenericHook ${this.roomId} ${this.hookId}`);
-        let content: {plain: string, html?: string};
+    public executeTransformationFunction(data: Record<string, unknown>): {plain: string, html?: string}|null {
         if (!this.transformationFunction) {
-            content = this.transformHookData(data);
-        } else {
-            try {
+            throw Error('Transformation function not defined');
+        }
         const vm = new NodeVM({
             console: 'off',
             wrapper: 'none',
@@ -209,14 +213,53 @@ export class GenericHookConnection extends BaseConnection implements IConnection
             eval: false,
             timeout: TRANSFORMATION_TIMEOUT_MS,
         });
+        vm.setGlobal('HookshotApiVersion', 'v2');
         vm.setGlobal('data', data);
         vm.run(this.transformationFunction);
-                content = vm.getGlobal('result');
-                if (typeof content === "string") {
-                    content = {plain: `Received webhook: ${content}`};
-                } else {
-                    content = {plain: `No content`};
+        const result = vm.getGlobal('result');
+
+        // Legacy v1 api
+        if (typeof result === "string") {
+            return {plain: `Received webhook: ${result}`};
+        } else if (typeof result !== "object") {
+            return {plain: `No content`};
         }
+        const transformationResult = result as WebhookTransformationResult;
+        if (transformationResult.version !== "v2") {
+            throw Error("Result returned from transformation didn't specify version = v2");
+        }
+
+        if (transformationResult.empty) {
+            return null; // No-op
+        }
+
+        const plain = transformationResult.plain;
+        if (typeof plain !== "string") {
+            throw Error("Result returned from transformation didn't provide a string value for plain");
+        }
+        if (transformationResult.html && typeof transformationResult.html !== "string") {
+            throw Error("Result returned from transformation didn't provide a string value for html");
+        }
+
+        return {
+            plain: plain,
+            html: transformationResult.html,
+        }
+    }
+
+    public async onGenericHook(data: Record<string, unknown>) {
+        log.info(`onGenericHook ${this.roomId} ${this.hookId}`);
+        let content: {plain: string, html?: string};
+        if (!this.transformationFunction) {
+            content = this.transformHookData(data);
+        } else {
+            try {
+                const potentialContent = this.executeTransformationFunction(data);
+                if (potentialContent === null) {
+                    // Explitly no action
+                    return;
+                }
+                content = potentialContent;
             } catch (ex) {
                 log.warn(`Failed to run transformation function`, ex);
                 content = {plain: `Webhook received but failed to process via transformation function`};

tests/connections/GenericHookTest.ts

@@ -8,7 +8,8 @@ import { AppserviceMock } from "../utils/AppserviceMock";
 
 const ROOM_ID = "!foo:bar";
 
-const TFFunction = "result = `The answer to '${data.question}' is ${data.answer}`;";
+const V1TFFunction = "result = `The answer to '${data.question}' is ${data.answer}`;";
+const V2TFFunction = "result = {plain: `The answer to '${data.question}' is ${data.answer}`, version: 'v2'}";
 
 function createGenericHook(state: GenericHookConnectionState = {
     name: "some-name"
@@ -109,9 +110,9 @@ describe("GenericHookConnection", () => {
             type: 'm.room.message',
         });
     });
-    it("will handle a hook event with a transformation function", async () => {
+    it("will handle a hook event with a v1 transformation function", async () => {
         const webhookData = {question: 'What is the meaning of life?', answer: 42};
-        const [connection, mq] = createGenericHook({name: 'test', transformationFunction: TFFunction}, {
+        const [connection, mq] = createGenericHook({name: 'test', transformationFunction: V1TFFunction}, {
                 enabled: true,
                 urlPrefix: "https://example.com/webhookurl",
                 allowJsTransformationFunctions: true,
@@ -132,6 +133,29 @@ describe("GenericHookConnection", () => {
             type: 'm.room.message',
         });
     });
+    it("will handle a hook event with a v2 transformation function", async () => {
+        const webhookData = {question: 'What is the meaning of life?', answer: 42};
+        const [connection, mq] = createGenericHook({name: 'test', transformationFunction: V2TFFunction}, {
+                enabled: true,
+                urlPrefix: "https://example.com/webhookurl",
+                allowJsTransformationFunctions: true,
+            }
+        );
+        const messagePromise = handleMessage(mq);
+        await connection.onGenericHook(webhookData);
+        expect(await messagePromise).to.deep.equal({
+            roomId: ROOM_ID,
+            sender: connection.getUserId(),
+            content: {
+                body: "The answer to 'What is the meaning of life?' is 42",
+                format: "org.matrix.custom.html",
+                formatted_body: "The answer to 'What is the meaning of life?' is 42",
+                msgtype: "m.notice",
+                "uk.half-shot.hookshot.webhook_data": webhookData,
+            },
+            type: 'm.room.message',
+        });
+    });
     it("will fail to handle a webhook with an invalid script", async () => {
         const webhookData = {question: 'What is the meaning of life?', answer: 42};
         const [connection, mq] = createGenericHook({name: 'test', transformationFunction: "bibble bobble"}, {