V13: Log webhook firing exceptions when they happen (#15393)

* Refactor Webhook logging to handle exceptions

* Add Exception occured property to WebhookLog.

src/Umbraco.Core/Models/WebhookLog.cs

@@ -25,4 +25,6 @@ public class WebhookLog
     public string ResponseHeaders { get; set; } = string.Empty;
 
     public string ResponseBody { get; set; } = string.Empty;
+
+    public bool ExceptionOccured { get; set; }
 }

src/Umbraco.Core/Models/WebhookResponseModel.cs

@@ -1,8 +0,0 @@
-namespace Umbraco.Cms.Core.Models;
-
-public class WebhookResponseModel
-{
-    public HttpResponseMessage? HttpResponseMessage { get; set; }
-
-    public int RetryCount { get; set; }
-}

src/Umbraco.Core/Services/IWebhookLogFactory.cs

@@ -1,9 +1,15 @@
 using Umbraco.Cms.Core.Models;
-using Umbraco.Cms.Core.Webhooks;
 
 namespace Umbraco.Cms.Core.Services;
 
 public interface IWebhookLogFactory
 {
-    Task<WebhookLog> CreateAsync(string eventAlias, WebhookResponseModel responseModel, IWebhook webhook, CancellationToken cancellationToken);
+    Task<WebhookLog> CreateAsync(
+        string eventAlias,
+        HttpRequestMessage requestMessage,
+        HttpResponseMessage? httpResponseMessage,
+        int retryCount,
+        Exception? exception,
+        IWebhook webhook,
+        CancellationToken cancellationToken);
 }

src/Umbraco.Core/Services/WebhookLogFactory.cs

@@ -6,7 +6,7 @@ namespace Umbraco.Cms.Core.Services;
 
 public class WebhookLogFactory : IWebhookLogFactory
 {
-    public async Task<WebhookLog> CreateAsync(string eventAlias, WebhookResponseModel responseModel, IWebhook webhook, CancellationToken cancellationToken)
+    public async Task<WebhookLog> CreateAsync(string eventAlias, HttpRequestMessage requestMessage, HttpResponseMessage? httpResponseMessage, int retryCount, Exception? exception, IWebhook webhook, CancellationToken cancellationToken)
     {
         var log = new WebhookLog
         {
@@ -15,20 +15,34 @@ public class WebhookLogFactory : IWebhookLogFactory
             Key = Guid.NewGuid(),
             Url = webhook.Url,
             WebhookKey = webhook.Key,
-            RetryCount = responseModel.RetryCount,
+            RetryCount = retryCount,
+            RequestHeaders = requestMessage.Headers.ToString(),
+            RequestBody = await requestMessage.Content?.ReadAsStringAsync(cancellationToken)!,
+            ExceptionOccured = exception is not null,
         };
 
-        if (responseModel.HttpResponseMessage is not null)
+        if (httpResponseMessage is not null)
         {
-            if (responseModel.HttpResponseMessage.RequestMessage?.Content is not null)
+            log.StatusCode = MapStatusCodeToMessage(httpResponseMessage.StatusCode);
+            log.ResponseHeaders = httpResponseMessage.Headers.ToString();
+            log.ResponseBody = await httpResponseMessage.Content.ReadAsStringAsync(cancellationToken);
+        }
+        else if (exception is HttpRequestException httpRequestException)
+        {
+            if (httpRequestException.StatusCode is not null)
             {
-                log.RequestBody = await responseModel.HttpResponseMessage.RequestMessage.Content.ReadAsStringAsync(cancellationToken);
-                log.RequestHeaders = CalculateHeaders(responseModel.HttpResponseMessage);
+                log.StatusCode = MapStatusCodeToMessage(httpRequestException.StatusCode.Value);
+            }
+            else
+            {
+                log.StatusCode = httpRequestException.HttpRequestError.ToString();
             }
 
-            log.ResponseBody = await responseModel.HttpResponseMessage.Content.ReadAsStringAsync(cancellationToken);
-            log.ResponseHeaders = responseModel.HttpResponseMessage.Headers.ToString();
-            log.StatusCode = MapStatusCodeToMessage(responseModel.HttpResponseMessage.StatusCode);
+            log.ResponseBody = $"{httpRequestException.HttpRequestError}: {httpRequestException.Message}";
+        }
+        else if (exception is not null)
+        {
+            log.ResponseBody = exception.Message;
         }
 
         return log;

src/Umbraco.Infrastructure/BackgroundJobs/Jobs/WebhookFiring.cs

@@ -97,6 +97,7 @@ public class WebhookFiring : IRecurringBackgroundJob
         };
 
         HttpResponseMessage? response = null;
+        Exception? exception = null;
         try
         {
             // Add headers
@@ -117,16 +118,11 @@ public class WebhookFiring : IRecurringBackgroundJob
         }
         catch (Exception ex)
         {
+            exception = ex;
             _logger.LogError(ex, "Error while sending webhook request for webhook {WebhookKey}.", webhook.Key);
         }
 
-        var webhookResponseModel = new WebhookResponseModel
-        {
-            HttpResponseMessage = response,
-            RetryCount = retryCount,
-        };
-
-        WebhookLog log = await _webhookLogFactory.CreateAsync(eventName, webhookResponseModel, webhook, cancellationToken);
+        WebhookLog log = await _webhookLogFactory.CreateAsync(eventName, request, response, retryCount, exception, webhook, cancellationToken);
         await _webhookLogService.CreateAsync(log);
 
         return response;

src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs

@@ -104,5 +104,6 @@ public class UmbracoPlan : MigrationPlan
         To<V_13_0_0.ChangeLogStatusCode>("{7DDCE198-9CA4-430C-8BBC-A66D80CA209F}");
         To<V_13_0_0.ChangeWebhookRequestObjectColumnToNvarcharMax>("{F74CDA0C-7AAA-48C8-94C6-C6EC3C06F599}");
         To<V_13_0_0.ChangeWebhookUrlColumnsToNvarcharMax>("{21C42760-5109-4C03-AB4F-7EA53577D1F5}");
+        To<V_13_0_0.AddExceptionOccured>("{6158F3A3-4902-4201-835E-1ED7F810B2D8}");
     }
 }

src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/AddExceptionOccured.cs

@@ -0,0 +1,25 @@
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Infrastructure.Persistence.Dtos;
+
+namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_13_0_0;
+
+public class AddExceptionOccured : MigrationBase
+{
+    public AddExceptionOccured(IMigrationContext context) : base(context)
+    {
+    }
+
+    protected override void Migrate()
+    {
+        if (ColumnExists(Constants.DatabaseSchema.Tables.WebhookLog, "exceptionOccured") == false)
+        {
+            // Use a custom SQL query to prevent selecting explicit columns (sortOrder doesn't exist yet)
+            List<WebhookLogDto> webhookLogDtos = Database.Fetch<WebhookLogDto>($"SELECT * FROM {Constants.DatabaseSchema.Tables.WebhookLog}");
+
+            Delete.Table(Constants.DatabaseSchema.Tables.WebhookLog).Do();
+            Create.Table<WebhookLogDto>().Do();
+
+            Database.InsertBatch(webhookLogDtos);
+        }
+    }
+}

src/Umbraco.Infrastructure/Persistence/Dtos/WebhookLogDto.cs

@@ -61,4 +61,7 @@ internal class WebhookLogDto
     [SpecialDbType(SpecialDbTypes.NVARCHARMAX)]
     [NullSetting(NullSetting = NullSettings.NotNull)]
     public string ResponseBody { get; set; } = string.Empty;
+
+    [Column(Name = "exceptionOccured")]
+    public bool ExceptionOccured { get; set; }
 }

src/Umbraco.Infrastructure/Persistence/Factories/WebhookLogFactory.cs

@@ -21,6 +21,7 @@ internal static class WebhookLogFactory
             RequestHeaders = log.RequestHeaders,
             ResponseHeaders = log.ResponseHeaders,
             WebhookKey = log.WebhookKey,
+            ExceptionOccured = log.ExceptionOccured,
         };
 
     public static WebhookLog DtoToEntity(WebhookLogDto dto) =>
@@ -38,5 +39,6 @@ internal static class WebhookLogFactory
             RequestHeaders = dto.RequestHeaders,
             ResponseHeaders = dto.ResponseHeaders,
             WebhookKey = dto.WebhookKey,
+            ExceptionOccured = dto.ExceptionOccured,
         };
 }

src/Umbraco.Web.BackOffice/Mapping/WebhookMapDefinition.cs

@@ -47,5 +47,6 @@ public class WebhookMapDefinition : IMapDefinition
         target.RequestHeaders = source.RequestHeaders;
         target.ResponseHeaders = source.ResponseHeaders;
         target.WebhookKey = source.WebhookKey;
+        target.ExceptionOccured = source.ExceptionOccured;
     }
 }

src/Umbraco.Web.Common/Models/WebhookLogViewModel.cs

@@ -37,4 +37,7 @@ public class WebhookLogViewModel
 
     [DataMember(Name = "responseBody")]
     public string ResponseBody { get; set; } = string.Empty;
+
+    [DataMember(Name = "exceptionOccured")]
+    public bool ExceptionOccured { get; set; }
 }

src/Umbraco.Web.UI.Client/src/views/webhooks/overlays/details.html

@@ -1,69 +1,71 @@
-<div ng-controller="Umbraco.Editors.Webhooks.DetailsController as vm" class="headless-webhook-log-entry-details-overlay" style="margin-top: 20px;">
+<div ng-controller="Umbraco.Editors.Webhooks.DetailsController as vm" class="headless-webhook-log-entry-details-overlay"
+     style="margin-top: 20px;">
 
   <umb-editor-view>
 
     <umb-editor-header
-        name="model.title"
-        name-locked="true"
-        hide-alias="true"
-        hide-icon="true"
-        hide-description="true">
+      name="model.title"
+      name-locked="true"
+      hide-alias="true"
+      hide-icon="true"
+      hide-description="true">
     </umb-editor-header>
 
     <umb-editor-container>
 
-       <umb-box>
-          <umb-box-header title-key="general_general"></umb-box-header>
-          <umb-box-content class="block-form">
+      <umb-box>
+        <umb-box-header title-key="general_general"></umb-box-header>
+        <umb-box-content class="block-form">
 
-              <umb-control-group>
-                <div class="flex items-center">
-                  <div class="flx-g0 flx-s0" style="flex-basis: 40px;">
-                    <umb-checkmark checked="true" size="m" style="cursor: default" ng-if="model.log.statusCode === 'OK (200)'"></umb-checkmark>
-                    <umb-icon icon="icon-wrong" class="umb-checkmark umb-checkmark--m" style="cursor: default;" ng-if="model.log.statusCode !== 'OK (200)'"></umb-icon>
-                  </div>
-                  <div class="flx-g1 flx-s1 flx-b2">{{model.log.statusCode}}</div>
-                </div>
-              </umb-control-group>
+          <umb-control-group>
+            <div class="flex items-center">
+              <div class="flx-g0 flx-s0" style="flex-basis: 40px;">
+                <umb-checkmark checked="true" size="m" style="cursor: default"
+                               ng-if="model.log.statusCode === 'OK (200)'"></umb-checkmark>
+                <umb-icon icon="icon-wrong" class="umb-checkmark umb-checkmark--m" style="cursor: default;"
+                          ng-if="model.log.statusCode !== 'OK (200)'"></umb-icon>
+              </div>
+              <div class="flx-g1 flx-s1 flx-b2">{{model.log.statusCode}}</div>
+            </div>
+          </umb-control-group>
 
-              <umb-control-group label="@general_date">
-                  <div>{{model.log.formattedLogDate}}</div>
-              </umb-control-group>
+          <umb-control-group label="@general_date">
+            <div>{{model.log.formattedLogDate}}</div>
+          </umb-control-group>
 
-              <umb-control-group label="@general_url">
-                  <div>{{model.log.url}}</div>
-              </umb-control-group>
+          <umb-control-group label="@general_url">
+            <div>{{model.log.url}}</div>
+          </umb-control-group>
 
-              <umb-control-group label="Event">
-                <div>{{model.log.eventAlias}}</div>
-              </umb-control-group>
+          <umb-control-group label="Event">
+            <div>{{model.log.eventAlias}}</div>
+          </umb-control-group>
 
-              <umb-control-group label="Retry count">
-                <div>{{model.log.retryCount}}</div>
-              </umb-control-group>
+          <umb-control-group label="Retry count">
+            <div>{{model.log.retryCount}}</div>
+          </umb-control-group>
 
-          </umb-box-content>
+        </umb-box-content>
 
-        </umb-box>
-
-      <div ng-if="model.log.statusCode === 'OK (200)'">
-        <umb-box>
-          <umb-box-header title="Request"></umb-box-header>
-          <umb-box-content class="block-form">
-            <pre class="code">{{model.log.requestHeaders}}</pre>
-            <umb-code-snippet language="'JSON'" wrap="true">{{vm.formatData(model.log.requestBody) | json}}</umb-code-snippet>
-          </umb-box-content>
-        </umb-box>
+      </umb-box>
 
+      <umb-box>
+        <umb-box-header title="Request"></umb-box-header>
+        <umb-box-content class="block-form">
+          <pre class="code">{{model.log.requestHeaders}}</pre>
+          <umb-code-snippet language="'JSON'" wrap="true">{{vm.formatData(model.log.requestBody) | json}}
+          </umb-code-snippet>
+        </umb-box-content>
+      </umb-box>
         <umb-box>
           <umb-box-header title="Response"></umb-box-header>
           <umb-box-content class="block-form">
             <pre class="code">{{model.log.responseHeaders}}</pre>
-            <umb-code-snippet language="'TEXT'" wrap="true">{{vm.formatData(model.log.responseBody) | json}}</umb-code-snippet>
+            <umb-code-snippet language="'TEXT'" wrap="true">{{vm.formatData(model.log.responseBody) | json}}
+            </umb-code-snippet>
           </umb-box-content>
         </umb-box>
-      </div>
-      <div ng-if="model.log.statusCode !== 'OK (200)'">
+      <div ng-if="model.log.exceptionOccured">
         <umb-box>
           <umb-box-header title="Request failed"></umb-box-header>
           <umb-box-content class="block-form">
@@ -78,15 +80,15 @@
     </umb-editor-container>
 
     <umb-editor-footer>
-        <umb-editor-footer-content-right>
-            <umb-button
-                type="button"
-                button-style="link"
-                label-key="general_close"
-                shortcut="esc"
-                action="vm.close()">
-            </umb-button>
-        </umb-editor-footer-content-right>
+      <umb-editor-footer-content-right>
+        <umb-button
+          type="button"
+          button-style="link"
+          label-key="general_close"
+          shortcut="esc"
+          action="vm.close()">
+        </umb-button>
+      </umb-editor-footer-content-right>
     </umb-editor-footer>
 
   </umb-editor-view>