Refactor model violation reporters (#825)

* Add reporter for `Model::preventAccessingMissingAttributes()`

* Refactor model violation reporters

* Add `discardedAttributeViolationReporter`

* Delay sending to after the app has terminated

* Fix reporting the origin when sending delayed report

* Add some basic tests

* Fix tests on older Laravel versions

src/Sentry/Laravel/Integration.php

@@ -2,17 +2,13 @@
 
 namespace Sentry\Laravel;
 
-use Illuminate\Database\Eloquent\MissingAttributeException;
-use Illuminate\Database\Eloquent\Model;
-use Illuminate\Database\LazyLoadingViolationException;
 use Illuminate\Foundation\Configuration\Exceptions;
 use Illuminate\Routing\Route;
 use Sentry\EventHint;
 use Sentry\EventId;
 use Sentry\ExceptionMechanism;
-use Sentry\Laravel\Features\Concerns\ResolvesEventOrigin;
+use Sentry\Laravel\Integration\ModelViolations as ModelViolationReports;
 use Sentry\SentrySdk;
-use Sentry\Severity;
 use Sentry\Tracing\TransactionSource;
 use Throwable;
 use Sentry\Breadcrumb;
@@ -244,105 +240,43 @@ public static function captureUnhandledException(Throwable $throwable): ?EventId
     /**
      * Returns a callback that can be passed to `Model::handleMissingAttributeViolationUsing` to report missing attribute violations to Sentry.
      *
-     * @param callable|null $callback Optional callback to be called after the violation is reported to Sentry.
+     * @param callable|null $callback                 Optional callback to be called after the violation is reported to Sentry.
+     * @param bool          $suppressDuplicateReports Whether to suppress duplicate reports of the same violation.
+     * @param bool          $reportAfterResponse      Whether to delay sending the report to after the response has been sent.
      *
      * @return callable
      */
-    public static function missingAttributeViolationReporter(?callable $callback = null): callable
+    public static function missingAttributeViolationReporter(?callable $callback = null, bool $suppressDuplicateReports = true, bool $reportAfterResponse = true): callable
     {
-        return new class($callback) {
-            use ResolvesEventOrigin;
-
-            /** @var callable|null $callback */
-            private $callback;
-
-            public function __construct(?callable $callback)
-            {
-                $this->callback = $callback;
-            }
-
-            public function __invoke(Model $model, string $attribute): void
-            {
-                SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($model, $attribute) {
-                    $scope->setContext('violation', [
-                        'model'     => get_class($model),
-                        'attribute' => $attribute,
-                        'origin'    => $this->resolveEventOrigin(),
-                        'kind'      => 'missing_attribute',
-                    ]);
-
-                    SentrySdk::getCurrentHub()->captureEvent(
-                        tap(Event::createEvent(), static function (Event $event) {
-                            $event->setLevel(Severity::warning());
-                        }),
-                        EventHint::fromArray([
-                            'exception' => new MissingAttributeException($model, $attribute),
-                            'mechanism' => new ExceptionMechanism(ExceptionMechanism::TYPE_GENERIC, true),
-                        ])
-                    );
-                });
-
-                // Forward the violation to the next handler if there is one
-                if ($this->callback !== null) {
-                    call_user_func($this->callback, $model, $attribute);
-                }
-            }
-        };
+        return new ModelViolationReports\MissingAttributeModelViolationReporter($callback, $suppressDuplicateReports, $reportAfterResponse);
     }
 
     /**
      * Returns a callback that can be passed to `Model::handleLazyLoadingViolationUsing` to report lazy loading violations to Sentry.
      *
-     * @param callable|null $callback Optional callback to be called after the violation is reported to Sentry.
+     * @param callable|null $callback                 Optional callback to be called after the violation is reported to Sentry.
+     * @param bool          $suppressDuplicateReports Whether to suppress duplicate reports of the same violation.
+     * @param bool          $reportAfterResponse      Whether to delay sending the report to after the response has been sent.
      *
      * @return callable
      */
-    public static function lazyLoadingViolationReporter(?callable $callback = null): callable
+    public static function lazyLoadingViolationReporter(?callable $callback = null, bool $suppressDuplicateReports = true, bool $reportAfterResponse = true): callable
     {
-        return new class($callback) {
-            use ResolvesEventOrigin;
-
-            /** @var callable|null $callback */
-            private $callback;
-
-            public function __construct(?callable $callback)
-            {
-                $this->callback = $callback;
-            }
+        return new ModelViolationReports\LazyLoadingModelViolationReporter($callback, $suppressDuplicateReports, $reportAfterResponse);
+    }
 
-            public function __invoke(Model $model, string $relation): void
-            {
-                // Laravel uses these checks itself to not throw an exception if the model doesn't exist or was just created
-                // See: https://github.com/laravel/framework/blob/438d02d3a891ab4d73ffea2c223b5d37947b5e93/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L559-L561
-                if (!$model->exists || $model->wasRecentlyCreated) {
-                    return;
-                }
-
-                SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($model, $relation) {
-                    $scope->setContext('violation', [
-                        'model'    => get_class($model),
-                        'relation' => $relation,
-                        'origin'   => $this->resolveEventOriginAsString(),
-                        'kind'     => 'lazy_loading',
-                    ]);
-
-                    SentrySdk::getCurrentHub()->captureEvent(
-                        tap(Event::createEvent(), static function (Event $event) {
-                            $event->setLevel(Severity::warning());
-                        }),
-                        EventHint::fromArray([
-                            'exception' => new LazyLoadingViolationException($model, $relation),
-                            'mechanism' => new ExceptionMechanism(ExceptionMechanism::TYPE_GENERIC, true),
-                        ])
-                    );
-                });
-
-                // Forward the violation to the next handler if there is one
-                if ($this->callback !== null) {
-                    call_user_func($this->callback, $model, $relation);
-                }
-            }
-        };
+    /**
+     * Returns a callback that can be passed to `Model::handleDiscardedAttributeViolationUsing` to report discarded attribute violations to Sentry.
+     *
+     * @param callable|null $callback                 Optional callback to be called after the violation is reported to Sentry.
+     * @param bool          $suppressDuplicateReports Whether to suppress duplicate reports of the same violation.
+     * @param bool          $reportAfterResponse      Whether to delay sending the report to after the response has been sent.
+     *
+     * @return callable
+     */
+    public static function discardedAttributeViolationReporter(?callable $callback = null, bool $suppressDuplicateReports = true, bool $reportAfterResponse = true): callable
+    {
+        return new ModelViolationReports\DiscardedAttributeViolationReporter($callback, $suppressDuplicateReports, $reportAfterResponse);
     }
 
     /**

src/Sentry/Laravel/Integration/ModelViolations/DiscardedAttributeViolationReporter.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Sentry\Laravel\Integration\ModelViolations;
+
+use Exception;
+use Illuminate\Database\Eloquent\MassAssignmentException;
+use Illuminate\Database\Eloquent\Model;
+
+class DiscardedAttributeViolationReporter extends ModelViolationReporter
+{
+    protected function getViolationContext(Model $model, string $property): array
+    {
+        return [
+            'attribute' => $property,
+            'kind' => 'discarded_attribute',
+        ];
+    }
+
+    protected function getViolationException(Model $model, string $property): Exception
+    {
+        return new MassAssignmentException(sprintf(
+            'Add [%s] to fillable property to allow mass assignment on [%s].',
+            $property,
+            get_class($model)
+        ));
+    }
+}

src/Sentry/Laravel/Integration/ModelViolations/LazyLoadingModelViolationReporter.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Sentry\Laravel\Integration\ModelViolations;
+
+use Exception;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\LazyLoadingViolationException;
+
+class LazyLoadingModelViolationReporter extends ModelViolationReporter
+{
+    protected function shouldReport(Model $model, string $property): bool
+    {
+        // Laravel uses these checks itself to not throw an exception if the model doesn't exist or was just created
+        // See: https://github.com/laravel/framework/blob/438d02d3a891ab4d73ffea2c223b5d37947b5e93/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L559-L561
+        if (!$model->exists || $model->wasRecentlyCreated) {
+            return false;
+        }
+
+        return parent::shouldReport($model, $property);
+    }
+
+    protected function getViolationContext(Model $model, string $property): array
+    {
+        return [
+            'relation' => $property,
+            'kind' => 'lazy_loading',
+        ];
+    }
+
+    protected function getViolationException(Model $model, string $property): Exception
+    {
+        return new LazyLoadingViolationException($model, $property);
+    }
+}

src/Sentry/Laravel/Integration/ModelViolations/MissingAttributeModelViolationReporter.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace Sentry\Laravel\Integration\ModelViolations;
+
+use Exception;
+use Illuminate\Database\Eloquent\MissingAttributeException;
+use Illuminate\Database\Eloquent\Model;
+
+class MissingAttributeModelViolationReporter extends ModelViolationReporter
+{
+    protected function getViolationContext(Model $model, string $property): array
+    {
+        return [
+            'attribute' => $property,
+            'kind' => 'missing_attribute',
+        ];
+    }
+
+    protected function getViolationException(Model $model, string $property): Exception
+    {
+        return new MissingAttributeException($model, $property);
+    }
+}

src/Sentry/Laravel/Integration/ModelViolations/ModelViolationReporter.php

@@ -0,0 +1,103 @@
+<?php
+
+namespace Sentry\Laravel\Integration\ModelViolations;
+
+use Exception;
+use Illuminate\Database\Eloquent\Model;
+use Sentry\Event;
+use Sentry\EventHint;
+use Sentry\ExceptionMechanism;
+use Sentry\Laravel\Features\Concerns\ResolvesEventOrigin;
+use Sentry\SentrySdk;
+use Sentry\Severity;
+use Sentry\State\Scope;
+
+abstract class ModelViolationReporter
+{
+    use ResolvesEventOrigin;
+
+    /** @var callable|null $callback */
+    private $callback;
+
+    /** @var bool $suppressDuplicateReports */
+    private $suppressDuplicateReports;
+
+    /** @var bool $reportAfterResponse */
+    private $reportAfterResponse;
+
+    /** @var array<string, true> $reportedViolations */
+    private $reportedViolations = [];
+
+    public function __construct(?callable $callback, bool $suppressDuplicateReports, bool $reportAfterResponse)
+    {
+        $this->callback = $callback;
+        $this->suppressDuplicateReports = $suppressDuplicateReports;
+        $this->reportAfterResponse = $reportAfterResponse;
+    }
+
+    public function __invoke(Model $model, string $property): void
+    {
+        if (!$this->shouldReport($model, $property)) {
+            return;
+        }
+
+        $this->markAsReported($model, $property);
+
+        $origin = $this->resolveEventOrigin();
+
+        if ($this->reportAfterResponse) {
+            app()->terminating(function () use ($model, $property, $origin) {
+                $this->report($model, $property, $origin);
+            });
+        } else {
+            $this->report($model, $property, $origin);
+        }
+    }
+
+    abstract protected function getViolationContext(Model $model, string $property): array;
+
+    abstract protected function getViolationException(Model $model, string $property): Exception;
+
+    protected function shouldReport(Model $model, string $property): bool
+    {
+        if (!$this->suppressDuplicateReports) {
+            return true;
+        }
+
+        return !array_key_exists(get_class($model) . $property, $this->reportedViolations);
+    }
+
+    protected function markAsReported(Model $model, string $property): void
+    {
+        if (!$this->suppressDuplicateReports) {
+            return;
+        }
+
+        $this->reportedViolations[get_class($model) . $property] = true;
+    }
+
+    private function report(Model $model, string $property, $origin): void
+    {
+        SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($model, $property, $origin) {
+            $scope->setContext('violation', array_merge([
+                'model' => get_class($model),
+                'origin' => $origin,
+            ], $this->getViolationContext($model, $property)));
+
+            SentrySdk::getCurrentHub()->captureEvent(
+                tap(Event::createEvent(), static function (Event $event) {
+                    $event->setLevel(Severity::warning());
+                }),
+                EventHint::fromArray([
+                    'exception' => $this->getViolationException($model, $property),
+                    'mechanism' => new ExceptionMechanism(ExceptionMechanism::TYPE_GENERIC, true),
+                ])
+            );
+        });
+
+        // Forward the violation to the next handler if there is one
+        if ($this->callback !== null) {
+            call_user_func($this->callback, $model, $property);
+        }
+    }
+}