]> BookStack Code Mirror - bookstack/commitdiff
Added timeout and debugging statuses to webhooks
authorDan Brown <redacted>
Mon, 3 Jan 2022 19:42:48 +0000 (19:42 +0000)
committerDan Brown <redacted>
Mon, 3 Jan 2022 19:42:48 +0000 (19:42 +0000)
- Added a user-configurable timeout option to webhooks.
- Added webhook fields for last-call/error datetime, in addition to last
  error string, which are shown on  webhook edit view.

Related to #3122

13 files changed:
app/Actions/DispatchWebhookJob.php
app/Actions/Webhook.php
app/Http/Controllers/WebhookController.php
database/factories/Actions/WebhookFactory.php
database/migrations/2022_01_03_154041_add_webhooks_timeout_error_columns.php [new file with mode: 0644]
resources/lang/en/common.php
resources/lang/en/settings.php
resources/views/form/number.blade.php [new file with mode: 0644]
resources/views/settings/roles/form.blade.php [deleted file]
resources/views/settings/webhooks/create.blade.php
resources/views/settings/webhooks/edit.blade.php
resources/views/settings/webhooks/parts/form.blade.php
tests/Actions/WebhookCallTest.php

index 57cb2feabad6afc8b375070e8aaf3697f139e424..7cc530c1097821febbaf269737392f7613cc1c18 100644 (file)
@@ -72,21 +72,31 @@ class DispatchWebhookJob implements ShouldQueue
     {
         $themeResponse = Theme::dispatch(ThemeEvents::WEBHOOK_CALL_BEFORE, $this->event, $this->webhook, $this->detail);
         $webhookData = $themeResponse ?? $this->buildWebhookData();
     {
         $themeResponse = Theme::dispatch(ThemeEvents::WEBHOOK_CALL_BEFORE, $this->event, $this->webhook, $this->detail);
         $webhookData = $themeResponse ?? $this->buildWebhookData();
+        $lastError = null;
 
         try {
             $response = Http::asJson()
                 ->withOptions(['allow_redirects' => ['strict' => true]])
 
         try {
             $response = Http::asJson()
                 ->withOptions(['allow_redirects' => ['strict' => true]])
-                ->timeout(3)
+                ->timeout($this->webhook->timeout)
                 ->post($this->webhook->endpoint, $webhookData);
 
         } catch (\Exception $exception) {
                 ->post($this->webhook->endpoint, $webhookData);
 
         } catch (\Exception $exception) {
-            Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$exception->getMessage()}\"");
-            return;
+            $lastError = $exception->getMessage();
+            Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\"");
         }
 
         }
 
-        if ($response->failed()) {
+        if (isset($response) && $response->failed()) {
+            $lastError = "Response status from endpoint was {$response->status()}";
             Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with status {$response->status()}");
         }
             Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with status {$response->status()}");
         }
+
+        $this->webhook->last_called_at = now();
+        if ($lastError) {
+            $this->webhook->last_errored_at = now();
+            $this->webhook->last_error = $lastError;
+        }
+
+        $this->webhook->save();
     }
 
     protected function buildWebhookData(): array
     }
 
     protected function buildWebhookData(): array
index 2c0bd0f157612b89cbf10b4a7256a5ea16241705..72a67ad9201f14ab38a359f0efe98d950fb19add 100644 (file)
@@ -3,6 +3,7 @@
 namespace BookStack\Actions;
 
 use BookStack\Interfaces\Loggable;
 namespace BookStack\Actions;
 
 use BookStack\Interfaces\Loggable;
+use Carbon\Carbon;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
@@ -14,13 +15,22 @@ use Illuminate\Database\Eloquent\Relations\HasMany;
  * @property string     $endpoint
  * @property Collection $trackedEvents
  * @property bool       $active
  * @property string     $endpoint
  * @property Collection $trackedEvents
  * @property bool       $active
+ * @property int        $timeout
+ * @property string     $last_error
+ * @property Carbon     $last_called_at
+ * @property Carbon     $last_errored_at
  */
 class Webhook extends Model implements Loggable
 {
  */
 class Webhook extends Model implements Loggable
 {
-    protected $fillable = ['name', 'endpoint'];
+    protected $fillable = ['name', 'endpoint', 'timeout'];
 
     use HasFactory;
 
 
     use HasFactory;
 
+    protected $casts = [
+        'last_called_at'  => 'datetime',
+        'last_errored_at' => 'datetime',
+    ];
+
     /**
      * Define the tracked event relation a webhook.
      */
     /**
      * Define the tracked event relation a webhook.
      */
index eca3002c6dd7e2d4e5f0dad76b3088ccc8ba83d1..81e3b77925aa7d4827503126475d5196413756aa 100644 (file)
@@ -46,6 +46,7 @@ class WebhookController extends Controller
             'endpoint' => ['required', 'url', 'max:500'],
             'events'   => ['required', 'array'],
             'active'   => ['required'],
             'endpoint' => ['required', 'url', 'max:500'],
             'events'   => ['required', 'array'],
             'active'   => ['required'],
+            'timeout'  => ['required', 'integer', 'min:1', 'max:600'],
         ]);
 
         $webhook = new Webhook($validated);
         ]);
 
         $webhook = new Webhook($validated);
@@ -81,6 +82,7 @@ class WebhookController extends Controller
             'endpoint' => ['required', 'url', 'max:500'],
             'events'   => ['required', 'array'],
             'active'   => ['required'],
             'endpoint' => ['required', 'url', 'max:500'],
             'events'   => ['required', 'array'],
             'active'   => ['required'],
+            'timeout'  => ['required', 'integer', 'min:1', 'max:600'],
         ]);
 
         /** @var Webhook $webhook */
         ]);
 
         /** @var Webhook $webhook */
index 205156793f89880b8a8e868881f4925ade5bc122..662f64f8bc90a5856f1b9c3ec2ffd4ba700ad70a 100644 (file)
@@ -20,6 +20,7 @@ class WebhookFactory extends Factory
             'name'     => 'My webhook for ' . $this->faker->country(),
             'endpoint' => $this->faker->url,
             'active'   => true,
             'name'     => 'My webhook for ' . $this->faker->country(),
             'endpoint' => $this->faker->url,
             'active'   => true,
+            'timeout'  => 3,
         ];
     }
 }
         ];
     }
 }
diff --git a/database/migrations/2022_01_03_154041_add_webhooks_timeout_error_columns.php b/database/migrations/2022_01_03_154041_add_webhooks_timeout_error_columns.php
new file mode 100644 (file)
index 0000000..c7258d0
--- /dev/null
@@ -0,0 +1,38 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class AddWebhooksTimeoutErrorColumns extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('webhooks', function (Blueprint $table) {
+            $table->unsignedInteger('timeout')->default(3);
+            $table->text('last_error')->default('');
+            $table->timestamp('last_called_at')->nullable();
+            $table->timestamp('last_errored_at')->nullable();
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('webhooks', function (Blueprint $table) {
+            $table->dropColumn('timeout');
+            $table->dropColumn('last_error');
+            $table->dropColumn('last_called_at');
+            $table->dropColumn('last_errored_at');
+        });
+    }
+}
index 53db3cf409a027bd44043c71b8175377222b176f..013042134f3a5c985139d018f0fdad24dd20c39e 100644 (file)
@@ -74,6 +74,7 @@ return [
     'status' => 'Status',
     'status_active' => 'Active',
     'status_inactive' => 'Inactive',
     'status' => 'Status',
     'status_active' => 'Active',
     'status_inactive' => 'Inactive',
+    'never' => 'Never',
 
     // Header
     'header_menu_expand' => 'Expand Header Menu',
 
     // Header
     'header_menu_expand' => 'Expand Header Menu',
index 08d4e7294acb6082a1178935c34b8851a0fe8e8c..65e2e5264a93cce73e7a520d14acd3645a9be5bb 100755 (executable)
@@ -246,6 +246,7 @@ return [
     'webhooks_events_warning' => 'Keep in mind that these events will be triggered for all selected events, even if custom permissions are applied. Ensure that use of this webhook won\'t expose confidential content.',
     'webhooks_events_all' => 'All system events',
     'webhooks_name' => 'Webhook Name',
     'webhooks_events_warning' => 'Keep in mind that these events will be triggered for all selected events, even if custom permissions are applied. Ensure that use of this webhook won\'t expose confidential content.',
     'webhooks_events_all' => 'All system events',
     'webhooks_name' => 'Webhook Name',
+    'webhooks_timeout' => 'Webhook Request Timeout (Seconds)',
     'webhooks_endpoint' => 'Webhook Endpoint',
     'webhooks_active' => 'Webhook Active',
     'webhook_events_table_header' => 'Events',
     'webhooks_endpoint' => 'Webhook Endpoint',
     'webhooks_active' => 'Webhook Active',
     'webhook_events_table_header' => 'Events',
@@ -254,6 +255,11 @@ return [
     'webhooks_delete_confirm' => 'Are you sure you want to delete this webhook?',
     'webhooks_format_example' => 'Webhook Format Example',
     'webhooks_format_example_desc' => 'Webhook data is sent as a POST request to the configured endpoint as JSON following the format below. The "related_item" and "url" properties are optional and will depend on the type of event triggered.',
     'webhooks_delete_confirm' => 'Are you sure you want to delete this webhook?',
     'webhooks_format_example' => 'Webhook Format Example',
     'webhooks_format_example_desc' => 'Webhook data is sent as a POST request to the configured endpoint as JSON following the format below. The "related_item" and "url" properties are optional and will depend on the type of event triggered.',
+    'webhooks_status' => 'Webhook Status',
+    'webhooks_last_called' => 'Last Called:',
+    'webhooks_last_errored' => 'Last Errored:',
+    'webhooks_last_error_message' => 'Last Error Message:',
+
 
     //! If editing translations files directly please ignore this in all
     //! languages apart from en. Content will be auto-copied from en.
 
     //! If editing translations files directly please ignore this in all
     //! languages apart from en. Content will be auto-copied from en.
diff --git a/resources/views/form/number.blade.php b/resources/views/form/number.blade.php
new file mode 100644 (file)
index 0000000..a37cd36
--- /dev/null
@@ -0,0 +1,12 @@
+<input type="number" id="{{ $name }}" name="{{ $name }}"
+       @if($errors->has($name)) class="text-neg" @endif
+       @if(isset($placeholder)) placeholder="{{$placeholder}}" @endif
+       @if($autofocus ?? false) autofocus @endif
+       @if($disabled ?? false) disabled="disabled" @endif
+       @if($readonly ?? false) readonly="readonly" @endif
+       @if($min ?? false) min="{{ $min }}" @endif
+       @if($max ?? false) max="{{ $max }}" @endif
+       @if(isset($model) || old($name)) value="{{ old($name) ? old($name) : $model->$name}}" @endif>
+@if($errors->has($name))
+    <div class="text-neg text-small">{{ $errors->first($name) }}</div>
+@endif
diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php
deleted file mode 100644 (file)
index e69de29..0000000
index 4f20dd07774be933201cbd2f0a85cedef174ada7..f7a99c7259d05964e6147ea964596a2992c4fb56 100644 (file)
@@ -8,9 +8,19 @@
             @include('settings.parts.navbar', ['selected' => 'webhooks'])
         </div>
 
             @include('settings.parts.navbar', ['selected' => 'webhooks'])
         </div>
 
-        <form action="{{ url("/settings/webhooks/create") }}" method="POST">
-            @include('settings.webhooks.parts.form', ['title' => trans('settings.webhooks_create')])
-        </form>
+        <div class="card content-wrap auto-height">
+            <h1 class="list-heading">{{ trans('settings.webhooks_create') }}</h1>
+
+            <form action="{{ url("/settings/webhooks/create") }}" method="POST">
+                {!! csrf_field() !!}
+                @include('settings.webhooks.parts.form', ['title' => trans('settings.webhooks_create')])
+
+                <div class="form-group text-right">
+                    <a href="{{ url("/settings/webhooks") }}" class="button outline">{{ trans('common.cancel') }}</a>
+                    <button type="submit" class="button">{{ trans('settings.webhooks_save') }}</button>
+                </div>
+            </form>
+        </div>
 
         @include('settings.webhooks.parts.format-example')
     </div>
 
         @include('settings.webhooks.parts.format-example')
     </div>
index 3b297eb7bf0bf7766fa23d48924b31edac7d95f2..27f3070ca653451d5f24bfc6a8571963fc868e9b 100644 (file)
@@ -7,10 +7,46 @@
             @include('settings.parts.navbar', ['selected' => 'webhooks'])
         </div>
 
             @include('settings.parts.navbar', ['selected' => 'webhooks'])
         </div>
 
-        <form action="{{ $webhook->getUrl() }}" method="POST">
-            {!! method_field('PUT') !!}
-            @include('settings.webhooks.parts.form', ['model' => $webhook, 'title' => trans('settings.webhooks_edit')])
-        </form>
+        <div class="card content-wrap auto-height">
+            <h1 class="list-heading">{{ trans('settings.webhooks_edit') }}</h1>
+
+
+            <div class="setting-list">
+            <div class="grid half">
+                <div>
+                    <label class="setting-list-label">{{ trans('settings.webhooks_status') }}</label>
+                    <p class="mb-none">
+                        {{ trans('settings.webhooks_last_called') }} {{ $webhook->last_called_at ? $webhook->last_called_at->diffForHumans() : trans('common.never') }}
+                        <br>
+                        {{ trans('settings.webhooks_last_errored') }} {{ $webhook->last_errored_at ? $webhook->last_errored_at->diffForHumans() : trans('common.never') }}
+                    </p>
+                </div>
+                <div class="text-muted">
+                    <br>
+                    @if($webhook->last_error)
+                        {{ trans('settings.webhooks_last_error_message') }} <br>
+                        <span class="text-warn text-small">{{ $webhook->last_error }}</span>
+                    @endif
+                </div>
+            </div>
+            </div>
+
+
+            <hr>
+
+            <form action="{{ $webhook->getUrl() }}" method="POST">
+                {!! csrf_field() !!}
+                {!! method_field('PUT') !!}
+                @include('settings.webhooks.parts.form', ['model' => $webhook, 'title' => trans('settings.webhooks_edit')])
+
+                <div class="form-group text-right">
+                    <a href="{{ url("/settings/webhooks") }}" class="button outline">{{ trans('common.cancel') }}</a>
+                    <a href="{{ $webhook->getUrl('/delete') }}" class="button outline">{{ trans('settings.webhooks_delete') }}</a>
+                    <button type="submit" class="button">{{ trans('settings.webhooks_save') }}</button>
+                </div>
+
+            </form>
+        </div>
 
         @include('settings.webhooks.parts.format-example')
     </div>
 
         @include('settings.webhooks.parts.format-example')
     </div>
index 458b6767bd7a82287ade261c54b67a974ccee944..c8592e266258494f9ac378617c90ffcf06566d9b 100644 (file)
@@ -1,75 +1,64 @@
-{!! csrf_field() !!}
+<div class="setting-list">
 
 
-<div class="card content-wrap auto-height">
-    <h1 class="list-heading">{{ $title }}</h1>
-
-    <div class="setting-list">
-
-        <div class="grid half">
+    <div class="grid half">
+        <div>
+            <label class="setting-list-label">{{ trans('settings.webhooks_details') }}</label>
+            <p class="small">{{ trans('settings.webhooks_details_desc') }}</p>
             <div>
             <div>
-                <label class="setting-list-label">{{ trans('settings.webhooks_details') }}</label>
-                <p class="small">{{ trans('settings.webhooks_details_desc') }}</p>
-                <div>
-                    @include('form.toggle-switch', [
-                        'name' => 'active',
-                        'value' => old('active') ?? $model->active ?? true,
-                        'label' => trans('settings.webhooks_active'),
-                    ])
-                    @include('form.errors', ['name' => 'active'])
-                </div>
+                @include('form.toggle-switch', [
+                    'name' => 'active',
+                    'value' => old('active') ?? $model->active ?? true,
+                    'label' => trans('settings.webhooks_active'),
+                ])
+                @include('form.errors', ['name' => 'active'])
             </div>
             </div>
-            <div>
-                <div class="form-group">
-                    <label for="name">{{ trans('settings.webhooks_name') }}</label>
-                    @include('form.text', ['name' => 'name'])
-                </div>
-                <div class="form-group">
-                    <label for="endpoint">{{ trans('settings.webhooks_endpoint') }}</label>
-                    @include('form.text', ['name' => 'endpoint'])
-                </div>
+        </div>
+        <div>
+            <div class="form-group">
+                <label for="name">{{ trans('settings.webhooks_name') }}</label>
+                @include('form.text', ['name' => 'name'])
+            </div>
+            <div class="form-group">
+                <label for="endpoint">{{ trans('settings.webhooks_endpoint') }}</label>
+                @include('form.text', ['name' => 'endpoint'])
+            </div>
+            <div class="form-group">
+                <label for="endpoint">{{ trans('settings.webhooks_timeout') }}</label>
+                @include('form.number', ['name' => 'timeout', 'min' => 1, 'max' => 600])
             </div>
         </div>
             </div>
         </div>
+    </div>
 
 
-        <div component="webhook-events">
-            <label class="setting-list-label">{{ trans('settings.webhooks_events') }}</label>
-            @include('form.errors', ['name' => 'events'])
-
-            <p class="small">{{ trans('settings.webhooks_events_desc') }}</p>
-            <p class="text-warn small">{{ trans('settings.webhooks_events_warning') }}</p>
-
-            <div class="toggle-switch-list">
-                @include('form.custom-checkbox', [
-                    'name' => 'events[]',
-                    'value' => 'all',
-                    'label' => trans('settings.webhooks_events_all'),
-                    'checked' => old('events') ? in_array('all', old('events')) : (isset($webhook) ? $webhook->tracksEvent('all') : false),
-                ])
-            </div>
+    <div component="webhook-events">
+        <label class="setting-list-label">{{ trans('settings.webhooks_events') }}</label>
+        @include('form.errors', ['name' => 'events'])
 
 
-            <hr class="my-s">
+        <p class="small">{{ trans('settings.webhooks_events_desc') }}</p>
+        <p class="text-warn small">{{ trans('settings.webhooks_events_warning') }}</p>
 
 
-            <div class="dual-column-content toggle-switch-list">
-                @foreach(\BookStack\Actions\ActivityType::all() as $activityType)
-                    <div>
-                        @include('form.custom-checkbox', [
-                           'name' => 'events[]',
-                           'value' => $activityType,
-                           'label' => $activityType,
-                           'checked' => old('events') ? in_array($activityType, old('events')) : (isset($webhook) ? $webhook->tracksEvent($activityType) : false),
-                       ])
-                    </div>
-                @endforeach
-            </div>
+        <div class="toggle-switch-list">
+            @include('form.custom-checkbox', [
+                'name' => 'events[]',
+                'value' => 'all',
+                'label' => trans('settings.webhooks_events_all'),
+                'checked' => old('events') ? in_array('all', old('events')) : (isset($webhook) ? $webhook->tracksEvent('all') : false),
+            ])
         </div>
 
         </div>
 
-    </div>
+        <hr class="my-s">
 
 
-    <div class="form-group text-right">
-        <a href="{{ url("/settings/webhooks") }}" class="button outline">{{ trans('common.cancel') }}</a>
-        @if ($webhook->id ?? false)
-            <a href="{{ $webhook->getUrl('/delete') }}" class="button outline">{{ trans('settings.webhooks_delete') }}</a>
-        @endif
-        <button type="submit" class="button">{{ trans('settings.webhooks_save') }}</button>
+        <div class="dual-column-content toggle-switch-list">
+            @foreach(\BookStack\Actions\ActivityType::all() as $activityType)
+                <div>
+                    @include('form.custom-checkbox', [
+                       'name' => 'events[]',
+                       'value' => $activityType,
+                       'label' => $activityType,
+                       'checked' => old('events') ? in_array($activityType, old('events')) : (isset($webhook) ? $webhook->tracksEvent($activityType) : false),
+                   ])
+                </div>
+            @endforeach
+        </div>
     </div>
 
     </div>
 
-</div>
+</div>
\ No newline at end of file
index 920bc286454bf24c8f6d711d1edbd7c4792dc5e4..d9f9ddad591cd61ea26f5953e22f0bf827b003ae 100644 (file)
@@ -53,11 +53,16 @@ class WebhookCallTest extends TestCase
         Http::fake([
             '*' => Http::response('', 500),
         ]);
         Http::fake([
             '*' => Http::response('', 500),
         ]);
-        $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
+        $webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
+        $this->assertNull($webhook->last_errored_at);
 
         $this->runEvent(ActivityType::ROLE_CREATE);
 
         $this->assertTrue($logger->hasError('Webhook call to endpoint https://wh.example.com failed with status 500'));
 
         $this->runEvent(ActivityType::ROLE_CREATE);
 
         $this->assertTrue($logger->hasError('Webhook call to endpoint https://wh.example.com failed with status 500'));
+
+        $webhook->refresh();
+        $this->assertEquals('Response status from endpoint was 500', $webhook->last_error);
+        $this->assertNotNull($webhook->last_errored_at);
     }
 
     public function test_webhook_call_exception_is_caught_and_logged()
     }
 
     public function test_webhook_call_exception_is_caught_and_logged()
@@ -65,11 +70,16 @@ class WebhookCallTest extends TestCase
         Http::shouldReceive('asJson')->andThrow(new \Exception('Failed to perform request'));
 
         $logger = $this->withTestLogger();
         Http::shouldReceive('asJson')->andThrow(new \Exception('Failed to perform request'));
 
         $logger = $this->withTestLogger();
-        $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
+        $webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
+        $this->assertNull($webhook->last_errored_at);
 
         $this->runEvent(ActivityType::ROLE_CREATE);
 
         $this->assertTrue($logger->hasError('Webhook call to endpoint https://wh.example.com failed with error "Failed to perform request"'));
 
         $this->runEvent(ActivityType::ROLE_CREATE);
 
         $this->assertTrue($logger->hasError('Webhook call to endpoint https://wh.example.com failed with error "Failed to perform request"'));
+
+        $webhook->refresh();
+        $this->assertEquals('Failed to perform request', $webhook->last_error);
+        $this->assertNotNull($webhook->last_errored_at);
     }
 
     public function test_webhook_call_data_format()
     }
 
     public function test_webhook_call_data_format()