Skip to content

Commit fefacb9

Browse files
stayallivestayallive
authored
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
1 parent 300bb6f commit fefacb9

File tree

6 files changed

+281
-90
lines changed

6 files changed

+281
-90
lines changed

‎src/Sentry/Laravel/Integration.php

Lines changed: 24 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,13 @@
22

33
namespace Sentry\Laravel;
44

5-
use Illuminate\Database\Eloquent\MissingAttributeException;
6-
use Illuminate\Database\Eloquent\Model;
7-
use Illuminate\Database\LazyLoadingViolationException;
85
use Illuminate\Foundation\Configuration\Exceptions;
96
use Illuminate\Routing\Route;
107
use Sentry\EventHint;
118
use Sentry\EventId;
129
use Sentry\ExceptionMechanism;
13-
use Sentry\Laravel\Features\Concerns\ResolvesEventOrigin;
10+
use Sentry\Laravel\Integration\ModelViolations as ModelViolationReports;
1411
use Sentry\SentrySdk;
15-
use Sentry\Severity;
1612
use Sentry\Tracing\TransactionSource;
1713
use Throwable;
1814
use Sentry\Breadcrumb;
@@ -244,105 +240,43 @@ public static function captureUnhandledException(Throwable $throwable): ?EventId
244240
/**
245241
* Returns a callback that can be passed to `Model::handleMissingAttributeViolationUsing` to report missing attribute violations to Sentry.
246242
*
247-
* @param callable|null $callback Optional callback to be called after the violation is reported to Sentry.
243+
* @param callable|null $callback Optional callback to be called after the violation is reported to Sentry.
244+
* @param bool $suppressDuplicateReports Whether to suppress duplicate reports of the same violation.
245+
* @param bool $reportAfterResponse Whether to delay sending the report to after the response has been sent.
248246
*
249247
* @return callable
250248
*/
251-
public static function missingAttributeViolationReporter(?callable $callback = null): callable
249+
public static function missingAttributeViolationReporter(?callable $callback = null, bool $suppressDuplicateReports = true, bool $reportAfterResponse = true): callable
252250
{
253-
return new class($callback) {
254-
use ResolvesEventOrigin;
255-
256-
/** @var callable|null $callback */
257-
private $callback;
258-
259-
public function __construct(?callable $callback)
260-
{
261-
$this->callback = $callback;
262-
}
263-
264-
public function __invoke(Model $model, string $attribute): void
265-
{
266-
SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($model, $attribute) {
267-
$scope->setContext('violation', [
268-
'model' => get_class($model),
269-
'attribute' => $attribute,
270-
'origin' => $this->resolveEventOrigin(),
271-
'kind' => 'missing_attribute',
272-
]);
273-
274-
SentrySdk::getCurrentHub()->captureEvent(
275-
tap(Event::createEvent(), static function (Event $event) {
276-
$event->setLevel(Severity::warning());
277-
}),
278-
EventHint::fromArray([
279-
'exception' => new MissingAttributeException($model, $attribute),
280-
'mechanism' => new ExceptionMechanism(ExceptionMechanism::TYPE_GENERIC, true),
281-
])
282-
);
283-
});
284-
285-
// Forward the violation to the next handler if there is one
286-
if ($this->callback !== null) {
287-
call_user_func($this->callback, $model, $attribute);
288-
}
289-
}
290-
};
251+
return new ModelViolationReports\MissingAttributeModelViolationReporter($callback, $suppressDuplicateReports, $reportAfterResponse);
291252
}
292253

293254
/**
294255
* Returns a callback that can be passed to `Model::handleLazyLoadingViolationUsing` to report lazy loading violations to Sentry.
295256
*
296-
* @param callable|null $callback Optional callback to be called after the violation is reported to Sentry.
257+
* @param callable|null $callback Optional callback to be called after the violation is reported to Sentry.
258+
* @param bool $suppressDuplicateReports Whether to suppress duplicate reports of the same violation.
259+
* @param bool $reportAfterResponse Whether to delay sending the report to after the response has been sent.
297260
*
298261
* @return callable
299262
*/
300-
public static function lazyLoadingViolationReporter(?callable $callback = null): callable
263+
public static function lazyLoadingViolationReporter(?callable $callback = null, bool $suppressDuplicateReports = true, bool $reportAfterResponse = true): callable
301264
{
302-
return new class($callback) {
303-
use ResolvesEventOrigin;
304-
305-
/** @var callable|null $callback */
306-
private $callback;
307-
308-
public function __construct(?callable $callback)
309-
{
310-
$this->callback = $callback;
311-
}
265+
return new ModelViolationReports\LazyLoadingModelViolationReporter($callback, $suppressDuplicateReports, $reportAfterResponse);
266+
}
312267

313-
public function __invoke(Model $model, string $relation): void
314-
{
315-
// Laravel uses these checks itself to not throw an exception if the model doesn't exist or was just created
316-
// See: https://github.com/laravel/framework/blob/438d02d3a891ab4d73ffea2c223b5d37947b5e93/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L559-L561
317-
if (!$model->exists || $model->wasRecentlyCreated) {
318-
return;
319-
}
320-
321-
SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($model, $relation) {
322-
$scope->setContext('violation', [
323-
'model' => get_class($model),
324-
'relation' => $relation,
325-
'origin' => $this->resolveEventOriginAsString(),
326-
'kind' => 'lazy_loading',
327-
]);
328-
329-
SentrySdk::getCurrentHub()->captureEvent(
330-
tap(Event::createEvent(), static function (Event $event) {
331-
$event->setLevel(Severity::warning());
332-
}),
333-
EventHint::fromArray([
334-
'exception' => new LazyLoadingViolationException($model, $relation),
335-
'mechanism' => new ExceptionMechanism(ExceptionMechanism::TYPE_GENERIC, true),
336-
])
337-
);
338-
});
339-
340-
// Forward the violation to the next handler if there is one
341-
if ($this->callback !== null) {
342-
call_user_func($this->callback, $model, $relation);
343-
}
344-
}
345-
};
268+
/**
269+
* Returns a callback that can be passed to `Model::handleDiscardedAttributeViolationUsing` to report discarded attribute violations to Sentry.
270+
*
271+
* @param callable|null $callback Optional callback to be called after the violation is reported to Sentry.
272+
* @param bool $suppressDuplicateReports Whether to suppress duplicate reports of the same violation.
273+
* @param bool $reportAfterResponse Whether to delay sending the report to after the response has been sent.
274+
*
275+
* @return callable
276+
*/
277+
public static function discardedAttributeViolationReporter(?callable $callback = null, bool $suppressDuplicateReports = true, bool $reportAfterResponse = true): callable
278+
{
279+
return new ModelViolationReports\DiscardedAttributeViolationReporter($callback, $suppressDuplicateReports, $reportAfterResponse);
346280
}
347281

348282
/**

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
namespace Sentry\Laravel\Integration\ModelViolations;
4+
5+
use Exception;
6+
use Illuminate\Database\Eloquent\MassAssignmentException;
7+
use Illuminate\Database\Eloquent\Model;
8+
9+
class DiscardedAttributeViolationReporter extends ModelViolationReporter
10+
{
11+
protected function getViolationContext(Model $model, string $property): array
12+
{
13+
return [
14+
'attribute' => $property,
15+
'kind' => 'discarded_attribute',
16+
];
17+
}
18+
19+
protected function getViolationException(Model $model, string $property): Exception
20+
{
21+
return new MassAssignmentException(sprintf(
22+
'Add [%s] to fillable property to allow mass assignment on [%s].',
23+
$property,
24+
get_class($model)
25+
));
26+
}
27+
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Sentry\Laravel\Integration\ModelViolations;
4+
5+
use Exception;
6+
use Illuminate\Database\Eloquent\Model;
7+
use Illuminate\Database\LazyLoadingViolationException;
8+
9+
class LazyLoadingModelViolationReporter extends ModelViolationReporter
10+
{
11+
protected function shouldReport(Model $model, string $property): bool
12+
{
13+
// Laravel uses these checks itself to not throw an exception if the model doesn't exist or was just created
14+
// See: https://github.com/laravel/framework/blob/438d02d3a891ab4d73ffea2c223b5d37947b5e93/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L559-L561
15+
if (!$model->exists || $model->wasRecentlyCreated) {
16+
return false;
17+
}
18+
19+
return parent::shouldReport($model, $property);
20+
}
21+
22+
protected function getViolationContext(Model $model, string $property): array
23+
{
24+
return [
25+
'relation' => $property,
26+
'kind' => 'lazy_loading',
27+
];
28+
}
29+
30+
protected function getViolationException(Model $model, string $property): Exception
31+
{
32+
return new LazyLoadingViolationException($model, $property);
33+
}
34+
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
namespace Sentry\Laravel\Integration\ModelViolations;
4+
5+
use Exception;
6+
use Illuminate\Database\Eloquent\MissingAttributeException;
7+
use Illuminate\Database\Eloquent\Model;
8+
9+
class MissingAttributeModelViolationReporter extends ModelViolationReporter
10+
{
11+
protected function getViolationContext(Model $model, string $property): array
12+
{
13+
return [
14+
'attribute' => $property,
15+
'kind' => 'missing_attribute',
16+
];
17+
}
18+
19+
protected function getViolationException(Model $model, string $property): Exception
20+
{
21+
return new MissingAttributeException($model, $property);
22+
}
23+
}

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
<?php
2+
3+
namespace Sentry\Laravel\Integration\ModelViolations;
4+
5+
use Exception;
6+
use Illuminate\Database\Eloquent\Model;
7+
use Sentry\Event;
8+
use Sentry\EventHint;
9+
use Sentry\ExceptionMechanism;
10+
use Sentry\Laravel\Features\Concerns\ResolvesEventOrigin;
11+
use Sentry\SentrySdk;
12+
use Sentry\Severity;
13+
use Sentry\State\Scope;
14+
15+
abstract class ModelViolationReporter
16+
{
17+
use ResolvesEventOrigin;
18+
19+
/** @var callable|null $callback */
20+
private $callback;
21+
22+
/** @var bool $suppressDuplicateReports */
23+
private $suppressDuplicateReports;
24+
25+
/** @var bool $reportAfterResponse */
26+
private $reportAfterResponse;
27+
28+
/** @var array<string, true> $reportedViolations */
29+
private $reportedViolations = [];
30+
31+
public function __construct(?callable $callback, bool $suppressDuplicateReports, bool $reportAfterResponse)
32+
{
33+
$this->callback = $callback;
34+
$this->suppressDuplicateReports = $suppressDuplicateReports;
35+
$this->reportAfterResponse = $reportAfterResponse;
36+
}
37+
38+
public function __invoke(Model $model, string $property): void
39+
{
40+
if (!$this->shouldReport($model, $property)) {
41+
return;
42+
}
43+
44+
$this->markAsReported($model, $property);
45+
46+
$origin = $this->resolveEventOrigin();
47+
48+
if ($this->reportAfterResponse) {
49+
app()->terminating(function () use ($model, $property, $origin) {
50+
$this->report($model, $property, $origin);
51+
});
52+
} else {
53+
$this->report($model, $property, $origin);
54+
}
55+
}
56+
57+
abstract protected function getViolationContext(Model $model, string $property): array;
58+
59+
abstract protected function getViolationException(Model $model, string $property): Exception;
60+
61+
protected function shouldReport(Model $model, string $property): bool
62+
{
63+
if (!$this->suppressDuplicateReports) {
64+
return true;
65+
}
66+
67+
return !array_key_exists(get_class($model) . $property, $this->reportedViolations);
68+
}
69+
70+
protected function markAsReported(Model $model, string $property): void
71+
{
72+
if (!$this->suppressDuplicateReports) {
73+
return;
74+
}
75+
76+
$this->reportedViolations[get_class($model) . $property] = true;
77+
}
78+
79+
private function report(Model $model, string $property, $origin): void
80+
{
81+
SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($model, $property, $origin) {
82+
$scope->setContext('violation', array_merge([
83+
'model' => get_class($model),
84+
'origin' => $origin,
85+
], $this->getViolationContext($model, $property)));
86+
87+
SentrySdk::getCurrentHub()->captureEvent(
88+
tap(Event::createEvent(), static function (Event $event) {
89+
$event->setLevel(Severity::warning());
90+
}),
91+
EventHint::fromArray([
92+
'exception' => $this->getViolationException($model, $property),
93+
'mechanism' => new ExceptionMechanism(ExceptionMechanism::TYPE_GENERIC, true),
94+
])
95+
);
96+
});
97+
98+
// Forward the violation to the next handler if there is one
99+
if ($this->callback !== null) {
100+
call_user_func($this->callback, $model, $property);
101+
}
102+
}
103+
}

0 commit comments

Comments
 (0)