Repository: getsentry/sentry-laravel (php)

Issue:
[Laravel Horizion] Breadcrumbs in Job handle function do not get recorded.
### Environment

Laravel: v9.44.0
Sentry: 3.12.0
Laravel-Sentry: 3.1.2
Sentry-SDK: 3.3.0

### Steps to Reproduce

Create a breadcrumb inside a job’s `handle` function.
```
\Sentry\Laravel\Integration::addBreadcrumb(new \Sentry\Breadcrumb(
    $level,
    $type,
    $category,
    $message,
    $data
));
```

Throw an exception inside the job’s `handle` function.

### Expected Result

Being able to see the breadcrumb inside the issue on Sentry.

### Actual Result

The breadcrumb does not appear.


Pull request:
Fix missing breadcrumbs for jobs throwing an exception
Fixes #632.

There is a downside with this "fix" (hence still a draft).

We might be leaking memory here since we never cleanup any breadcrumbs from failed queue jobs. When a queue workers is running this might be an issue where these breadcrumbs will never be visible but are kept in memory. So when a queue worker churns through say 500 jobs that threw exceptions it has 1500 breadcrumbs (3 per scope) and 500 scopes (unless we limit the amount of scopes already!?).

So not a 100% sure this is the way.

A fix for this can be where we pop the scope if we need to before we run a job, however that breaks [this](https://github.com/getsentry/sentry-laravel/pull/633/files#diff-212611d9baa058b4d98139c7480ad5b39c8f94a5425e2f6e6f5698815c422489R43-R51) test where the middle breadcrumb from this test is being lost which is not ideal either.

Patch:
diff --git a/src/Sentry/Laravel/EventHandler.php b/src/Sentry/Laravel/EventHandler.php
index f67b178..24bb1b2 100644
--- a/src/Sentry/Laravel/EventHandler.php
+++ b/src/Sentry/Laravel/EventHandler.php
@@ -409,6 +409,8 @@ class EventHandler
 
     protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): void
     {
+        $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScopeCount > 0);
+
         $this->prepareScopeForTaskWithinLongRunningProcess();
 
         ++$this->pushedQueueScopeCount;
@@ -440,8 +442,6 @@ class EventHandler
 
     protected function queueJobExceptionOccurredHandler(QueueEvents\JobExceptionOccurred $event): void
     {
-        $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScopeCount > 0);
-
         $this->afterTaskWithinLongRunningProcess();
     }
 


Test patch:
diff --git a/test/Sentry/EventHandler/QueueEventsTest.php b/test/Sentry/EventHandler/QueueEventsTest.php
new file mode 100644
index 0000000..1b06820
--- /dev/null
+++ b/test/Sentry/EventHandler/QueueEventsTest.php
@@ -0,0 +1,135 @@
+<?php
+
+namespace Sentry\Laravel\Tests\EventHandler;
+
+use Exception;
+use Illuminate\Contracts\Queue\ShouldQueue;
+use Sentry\Breadcrumb;
+use Sentry\Laravel\Tests\TestCase;
+use function Sentry\addBreadcrumb;
+use function Sentry\captureException;
+
+class QueueEventsTest extends TestCase
+{
+    public function testQueueJobPushesAndPopsScopeWithBreadcrumbs(): void
+    {
+        dispatch(new QueueEventsTestJobWithBreadcrumb);
+
+        $this->assertCount(0, $this->getCurrentBreadcrumbs());
+    }
+
+    public function testQueueJobThatReportsPushesAndPopsScopeWithBreadcrumbs(): void
+    {
+        dispatch(new QueueEventsTestJobThatReportsAnExceptionWithBreadcrumb);
+
+        $this->assertCount(0, $this->getCurrentBreadcrumbs());
+
+        $this->assertNotNull($this->getLastEvent());
+
+        $event = $this->getLastEvent();
+
+        $this->assertCount(2, $event->getBreadcrumbs());
+    }
+
+    public function testQueueJobThatThrowsLeavesPushedScopeWithBreadcrumbs(): void
+    {
+        try {
+            dispatch(new QueueEventsTestJobThatThrowsAnUnhandledExceptionWithBreadcrumb);
+        } catch (Exception $e) {
+            // No action required, expected to throw
+        }
+
+        // We still expect to find the breadcrumbs from the job here so they are attached to reported exceptions
+
+        $this->assertCount(2, $this->getCurrentBreadcrumbs());
+
+        $firstBreadcrumb = $this->getCurrentBreadcrumbs()[0];
+        $this->assertEquals('queue.job', $firstBreadcrumb->getCategory());
+
+        $secondBreadcrumb = $this->getCurrentBreadcrumbs()[1];
+        $this->assertEquals('test', $secondBreadcrumb->getCategory());
+    }
+
+    public function testQueueJobsThatThrowPopsAndPushesScopeWithBreadcrumbsBeforeNewJob(): void
+    {
+        try {
+            dispatch(new QueueEventsTestJobThatThrowsAnUnhandledExceptionWithBreadcrumb('test #1'));
+        } catch (Exception $e) {
+            // No action required, expected to throw
+        }
+
+        try {
+            dispatch(new QueueEventsTestJobThatThrowsAnUnhandledExceptionWithBreadcrumb('test #2'));
+        } catch (Exception $e) {
+            // No action required, expected to throw
+        }
+
+        // We only expect to find the breadcrumbs from the second job here
+
+        $this->assertCount(2, $this->getCurrentBreadcrumbs());
+
+        $firstBreadcrumb = $this->getCurrentBreadcrumbs()[0];
+        $this->assertEquals('queue.job', $firstBreadcrumb->getCategory());
+
+        $secondBreadcrumb = $this->getCurrentBreadcrumbs()[1];
+        $this->assertEquals('test #2', $secondBreadcrumb->getMessage());
+    }
+
+    public function testQueueJobsWithBreadcrumbSetInBetweenKeepsNonJobBreadcrumbsOnCurrentScope(): void
+    {
+        dispatch(new QueueEventsTestJobWithBreadcrumb);
+
+        addBreadcrumb(new Breadcrumb(Breadcrumb::LEVEL_INFO, Breadcrumb::LEVEL_DEBUG, 'test2', 'test2'));
+
+        dispatch(new QueueEventsTestJobWithBreadcrumb);
+
+        $this->assertCount(1, $this->getCurrentBreadcrumbs());
+    }
+}
+
+function queueEventsTestAddTestBreadcrumb($message = null): void
+{
+    addBreadcrumb(
+        new Breadcrumb(
+            Breadcrumb::LEVEL_INFO,
+            Breadcrumb::LEVEL_DEBUG,
+            'test',
+            $message ?? 'test'
+        )
+    );
+}
+
+class QueueEventsTestJobWithBreadcrumb implements ShouldQueue
+{
+    public function handle(): void
+    {
+        queueEventsTestAddTestBreadcrumb();
+    }
+}
+
+class QueueEventsTestJobThatReportsAnExceptionWithBreadcrumb implements ShouldQueue
+{
+    public function handle(): void
+    {
+        queueEventsTestAddTestBreadcrumb();
+
+        captureException(new Exception('This is a test exception'));
+    }
+}
+
+class QueueEventsTestJobThatThrowsAnUnhandledExceptionWithBreadcrumb implements ShouldQueue
+{
+    private $message;
+
+    public function __construct($message = null)
+    {
+        $this->message = $message;
+    }
+
+    public function handle(): void
+    {
+        queueEventsTestAddTestBreadcrumb($this->message);
+
+        throw new Exception('This is a test exception');
+    }
+}
diff --git a/test/Sentry/TestCase.php b/test/Sentry/TestCase.php
index 1449c2e..490ea51 100644
--- a/test/Sentry/TestCase.php
+++ b/test/Sentry/TestCase.php
@@ -86,6 +86,7 @@ abstract class TestCase extends LaravelTestCase
         return $method->invoke($hub);
     }
 
+    /** @return array<array-key, \Sentry\Breadcrumb> */
     protected function getCurrentBreadcrumbs(): array
     {
         $scope = $this->getCurrentScope();