diff --git a/changelog.d/1251.bugfix b/changelog.d/1251.bugfix new file mode 100644 index 000000000..c02344783 --- /dev/null +++ b/changelog.d/1251.bugfix @@ -0,0 +1 @@ +Fix generic webhooks returning HTTP error when waitForComplete is enabled with a transformation function. diff --git a/src/Bridge.ts b/src/Bridge.ts index 9edf61d0c..84df388ca 100644 --- a/src/Bridge.ts +++ b/src/Bridge.ts @@ -952,6 +952,22 @@ export class Bridge { event: "generic-webhook.event", connectionId: c.connectionId, }); + if ( + index === 0 && + !didPush && + (this.config.generic?.waitForComplete || c.waitForComplete) + ) { + await this.queue.push({ + data: { + successful: false, + error: ex instanceof Error ? ex.message : "Unknown error", + }, + sender: "Bridge", + messageId, + eventName: "response.generic-webhook.event", + }); + didPush = true; + } } }), ); diff --git a/src/Connections/GenericHook.ts b/src/Connections/GenericHook.ts index 702854068..a15cf170b 100644 --- a/src/Connections/GenericHook.ts +++ b/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); diff --git a/tests/connections/GenericHookTest.ts b/tests/connections/GenericHookTest.ts index 07db70ab3..81bfd4052 100644 --- a/tests/connections/GenericHookTest.ts +++ b/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,