Yncyrydybyl/fix waitforcomplete transformation

changelog.d/1251.bugfix

@@ -0,0 +1 @@
+Fix generic webhooks returning HTTP error when waitForComplete is enabled with a transformation function.

src/Bridge.ts

@@ -952,6 +952,22 @@
               event: "generic-webhook.event",
               connectionId: c.connectionId,
             });
+            if (
+              index === 0 &&
+              !didPush &&
+              (this.config.generic?.waitForComplete || c.waitForComplete)
+            ) {
+              await this.queue.push<GenericWebhookEventResult>({
+                data: {
+                  successful: false,
+                  error: ex instanceof Error ? ex.message : "Unknown error",
+                },
+                sender: "Bridge",
+                messageId,
+                eventName: "response.generic-webhook.event",
+              });
+              didPush = true;
+            }
           }
         }),
       );
@@ -1484,7 +1500,7 @@
       );
       // This might be a reply to a notification
       try {
         const evContent = replyEvent.content as any;
         const splitParts: string[] =
           evContent["uk.half-shot.matrix-hookshot.github.repo"]?.name.split(
             "/",

src/Connections/GenericHook.ts

@@ -652,7 +652,7 @@ export class GenericHookConnection
 
     let content: ExecuteResultContent | undefined;
     let webhookResponse: ExecuteResultWebhookResponse | undefined;
-    let successful = true;
+    const successful = true;
     if (this.webhookTransformer) {
       try {
         const result = this.webhookTransformer.execute(data);
@@ -663,7 +663,10 @@ export class GenericHookConnection
         content = {
           plain: `Webhook received but failed to process via transformation function`,
         };
-        successful = false;
+        // Don't set successful=false here — the webhook will still be
+        // delivered to the room with fallback text. Marking it as failed
+        // causes waitForComplete callers to receive an HTTP error even
+        // though the message was posted.
       }
     } else {
       content = this.transformHookData(data);

tests/connections/GenericHookTest.ts

@@ -476,6 +476,158 @@ describe("GenericHookConnection", () => {
     }
   });
 
+  it("will return successful when a valid transformation function is used", async () => {
+    const [connection, mq] = createGenericHook(
+      { name: "test", transformationFunction: V2TFFunction },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({
+      question: "What is the meaning of life?",
+      answer: 42,
+    });
+    await messagePromise;
+    expect(result.successful).to.be.true;
+  });
+
+  it("will return successful even when a transformation function throws", async () => {
+    const [connection, mq] = createGenericHook(
+      {
+        name: "test",
+        transformationFunction: "throw new Error('the spoon is a lie')",
+      },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({ test: "data" });
+    await messagePromise;
+    // The message is delivered with fallback text, so the webhook was
+    // processed successfully from the caller's perspective.
+    expect(result.successful).to.be.true;
+  });
+
+  it("will return successful when transformation function has invalid syntax", async () => {
+    const [connection, mq] = createGenericHook(
+      { name: "test", transformationFunction: "bibble bobble" },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({ test: "data" });
+    await messagePromise;
+    expect(result.successful).to.be.true;
+  });
+
+  it("will include webhookResponse in result when transformation provides one", async () => {
+    const tfWithResponse = `result = {
+      plain: "processed",
+      version: "v2",
+      webhookResponse: { body: '{"received": true}', statusCode: 200, contentType: "application/json" }
+    }`;
+    const [connection, mq] = createGenericHook(
+      { name: "test", transformationFunction: tfWithResponse },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({ test: "data" });
+    await messagePromise;
+    expect(result.successful).to.be.true;
+    expect(result).to.have.property("response");
+    const response = (result as { response: { body: string } }).response;
+    expect(response.body).to.equal('{"received": true}');
+  });
+
+  it("will not include webhookResponse when transformation has no webhookResponse", async () => {
+    const [connection, mq] = createGenericHook(
+      { name: "test", transformationFunction: V2TFFunction },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({
+      question: "What is the meaning of life?",
+      answer: 42,
+    });
+    await messagePromise;
+    expect(result.successful).to.be.true;
+    expect((result as { response?: unknown }).response).to.be.undefined;
+  });
+
+  it("will not include webhookResponse when transformation throws", async () => {
+    const [connection, mq] = createGenericHook(
+      { name: "test", transformationFunction: "throw new Error('kaboom')" },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({ test: "data" });
+    await messagePromise;
+    expect(result.successful).to.be.true;
+    expect((result as { response?: unknown }).response).to.be.undefined;
+  });
+
+  it("will handle empty result from transformation with webhookResponse", async () => {
+    const tfEmpty = `result = {
+      empty: true,
+      version: "v2",
+      webhookResponse: { body: '{"status": "ignored"}', statusCode: 204 }
+    }`;
+    const [connection] = createGenericHook(
+      { name: "test", transformationFunction: tfEmpty },
+      { allowJsTransformationFunctions: true },
+    );
+    // No message should be sent to Matrix since content is empty
+    const result = await connection.onGenericHook({ test: "data" });
+    expect(result.successful).to.be.true;
+    const response = (
+      result as { response: { body: string; statusCode: number } }
+    ).response;
+    expect(response.body).to.equal('{"status": "ignored"}');
+    expect(response.statusCode).to.equal(204);
+  });
+
+  it("will return successful:true for simple webhook without transformation", async () => {
+    const [connection, mq] = createGenericHook();
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({ simple: "data" });
+    await messagePromise;
+    expect(result.successful).to.be.true;
+    expect((result as { response?: unknown }).response).to.be.undefined;
+  });
+
+  it("will deliver fallback text to Matrix when transformation throws", async () => {
+    const [connection, mq] = createGenericHook(
+      {
+        name: "test",
+        transformationFunction: "this is not valid javascript!!!",
+      },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    await connection.onGenericHook({ test: "data" });
+    const message = await messagePromise;
+    expect(message.content.body).to.equal(
+      "Webhook received but failed to process via transformation function",
+    );
+  });
+
+  it("will handle transformation that returns v2 result with custom msgtype", async () => {
+    const tfCustomMsgtype = `result = {
+      plain: "a towel is the most massively useful thing",
+      msgtype: "m.text",
+      version: "v2"
+    }`;
+    const [connection, mq] = createGenericHook(
+      { name: "test", transformationFunction: tfCustomMsgtype },
+      { allowJsTransformationFunctions: true },
+    );
+    const messagePromise = handleMessage(mq);
+    const result = await connection.onGenericHook({ test: "data" });
+    const message = await messagePromise;
+    expect(result.successful).to.be.true;
+    expect(message.content.msgtype).to.equal("m.text");
+    expect(message.content.body).to.equal(
+      "a towel is the most massively useful thing",
+    );
+  });
+
   it("should fail to create a hook with an invalid expiry time", () => {
     for (const expirationDate of [
       0,