Merge pull request #1815 from Budibase/fix/webhook-issue

Fixing some issues discovered with webhooks sending JSON

packages/builder/src/components/automation/AutomationBuilder/FlowChart/FlowItem.svelte

@@ -14,7 +14,6 @@
   $: allowDeleteTrigger = !steps.length
 
   function deleteStep() {
-    console.log("Running")
     automationStore.actions.deleteAutomationBlock(block)
   }
 </script>

packages/builder/src/components/automation/Shared/CreateWebhookModal.svelte

@@ -16,11 +16,13 @@
   $: automation = $automationStore.selectedAutomation?.automation
 
   onMount(async () => {
+    if (!automation?.definition?.trigger?.inputs.schemaUrl) {
       // save the automation initially
       await automationStore.actions.save({
         instanceId,
         automation,
       })
+    }
     interval = setInterval(async () => {
       await automationStore.actions.fetch()
       const outputs = automation?.definition?.trigger.schema.outputs?.properties

packages/builder/src/components/automation/Shared/WebhookDisplay.svelte

@@ -9,6 +9,9 @@
   $: appUrl = $hostingStore.appUrl
 
   function fullWebhookURL(uri) {
+    if (!uri) {
+      return ""
+    }
     if (production) {
       return `${appUrl}/${uri}`
     } else {

packages/server/__mocks__/node-fetch.js

@@ -4,6 +4,11 @@ module.exports = async (url, opts) => {
   function json(body, status = 200) {
     return {
       status,
+      headers: {
+        get: () => {
+          return ["application/json"]
+        },
+      },
       json: async () => {
         return body
       },

packages/server/src/api/controllers/automation.js

@@ -88,6 +88,8 @@ async function checkForCronTriggers({ appId, oldAuto, newAuto }) {
 async function checkForWebhooks({ appId, oldAuto, newAuto }) {
   const oldTrigger = oldAuto ? oldAuto.definition.trigger : null
   const newTrigger = newAuto ? newAuto.definition.trigger : null
+  const triggerChanged =
+    oldTrigger && newTrigger && oldTrigger.id !== newTrigger.id
   function isWebhookTrigger(auto) {
     return (
       auto &&
@@ -98,9 +100,10 @@ async function checkForWebhooks({ appId, oldAuto, newAuto }) {
   // need to delete webhook
   if (
     isWebhookTrigger(oldAuto) &&
-    !isWebhookTrigger(newAuto) &&
+    (!isWebhookTrigger(newAuto) || triggerChanged) &&
     oldTrigger.webhookId
   ) {
+    try {
       let db = new CouchDB(appId)
       // need to get the webhook to get the rev
       const webhook = await db.get(oldTrigger.webhookId)
@@ -114,9 +117,15 @@ async function checkForWebhooks({ appId, oldAuto, newAuto }) {
         newTrigger.inputs = {}
       }
       await webhooks.destroy(ctx)
+    } catch (err) {
+      // don't worry about not being able to delete, if it doesn't exist all good
+    }
   }
   // need to create webhook
-  else if (!isWebhookTrigger(oldAuto) && isWebhookTrigger(newAuto)) {
+  if (
+    (!isWebhookTrigger(oldAuto) || triggerChanged) &&
+    isWebhookTrigger(newAuto)
+  ) {
     const ctx = {
       appId,
       request: {

packages/server/src/automations/steps/outgoingWebhook.js

@@ -77,14 +77,34 @@ module.exports.run = async function ({ inputs }) {
     requestBody.length !== 0 &&
     BODY_REQUESTS.indexOf(requestMethod) !== -1
   ) {
-    request.body = JSON.parse(requestBody)
+    request.body =
+      typeof requestBody === "string"
+        ? requestBody
+        : JSON.stringify(requestBody)
+    request.headers = {
+      "Content-Type": "application/json",
+    }
   }
 
   try {
+    // do a quick JSON parse if there is a body, to generate an error if its invalid
+    if (request.body) {
+      JSON.parse(request.body)
+    }
     const response = await fetch(url, request)
+    const contentType = response.headers.get("content-type")
+    const success = response.status === 200
+    let resp
+    if (!success) {
+      resp = response.statusText
+    } else if (contentType && contentType.indexOf("application/json") !== -1) {
+      resp = await response.json()
+    } else {
+      resp = await response.text()
+    }
     return {
-      response: await response.json(),
-      success: response.status === 200,
+      response: resp,
+      success: success,
     }
   } catch (err) {
     /* istanbul ignore next */

packages/server/src/automations/tests/outgoingWebhook.spec.js

@@ -25,7 +25,7 @@ describe("test the outgoing webhook action", () => {
     expect(res.success).toEqual(true)
     expect(res.response.url).toEqual("http://www.test.com")
     expect(res.response.method).toEqual("POST")
-    expect(res.response.body.a).toEqual(1)
+    expect(JSON.parse(res.response.body).a).toEqual(1)
   })
 
   it("should return an error if something goes wrong in fetch", async () => {