bug #43781 [Messenger] Fix TraceableMessageBus implementation so it can compute caller even when used within a callback (Ocramius)

nicolas-grekas · nicolas-grekas · commit 36f8fe585690 · 2021-10-27T18:01:49.000+02:00
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [Messenger] Fix `TraceableMessageBus` implementation so it can compute caller even when used within a callback | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | Quoting commits: > > This test shows that `array_map([new TraceableMessageBus(...), 'dispatch'], $messages)` does not work at all, > because `debug_backtrace()` is used in internals to compute the call-site, and it is not considering the fact > that the caller may be: > > * an internal PHP function > * a callback (this example test scenario) > * generated code (`eval()` result) > > The output of such a failure: > > ``` > 1) Symfony\Component\Messenger\Tests\TraceableMessageBusTest::testItTracesExceptionsWhenMessageBusIsFiredFromArrayCallback > Undefined array key "line" > > /app/src/Symfony/Component/Messenger/TraceableMessageBus.php:66 > /app/src/Symfony/Component/Messenger/TraceableMessageBus.php:36 > /app/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php:175 > ``` > > To be on the safe side, also removed `compact()` usage, which was making everything > much muddier and harder to comprehend. Commits ------- 1423229 [Messenger] Fix `TraceableMessageBus` implementation so it can compute caller even when used within a callback
diff --git a/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php b/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php
@@ -156,4 +156,22 @@ public function testItTracesExceptions()
             ],
         ], $actualTracedMessage);
     }
+
+    public function testItTracesExceptionsWhenMessageBusIsFiredFromArrayCallback()
+    {
+        $message = new DummyMessage('Hello');
+        $exception = new \RuntimeException();
+
+        $bus = $this->createMock(MessageBusInterface::class);
+        $bus->expects($this->once())
+            ->method('dispatch')
+            ->with($message)
+            ->willThrowException($exception);
+
+        $traceableBus = new TraceableMessageBus($bus);
+
+        $this->expectExceptionObject($exception);
+
+        array_map([$traceableBus, 'dispatch'], [$message]);
+    }
 }
diff --git a/src/Symfony/Component/Messenger/TraceableMessageBus.php b/src/Symfony/Component/Messenger/TraceableMessageBus.php
@@ -62,8 +62,8 @@ private function getCaller(): array
     {
         $trace = debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS, 8);
 
-        $file = $trace[1]['file'];
-        $line = $trace[1]['line'];
+        $file = $trace[1]['file'] ?? null;
+        $line = $trace[1]['line'] ?? null;
 
         $handleTraitFile = (new \ReflectionClass(HandleTrait::class))->getFileName();
         $found = false;
@@ -97,9 +97,12 @@ private function getCaller(): array
             }
         }
 
-        $name = str_replace('\\', '/', $file);
-        $name = substr($name, strrpos($name, '/') + 1);
+        $name = str_replace('\\', '/', (string) $file);
 
-        return compact('name', 'file', 'line');
+        return [
+            'name' => substr($name, strrpos($name, '/') + 1),
+            'file' => $file,
+            'line' => $line,
+        ];
     }
 }

Original file line number	Diff line number	Diff line 8000 change
`@@ -62,8 +62,8 @@ private function getCaller(): array`
`62`	`62`	`{`
`63`	`63`	`$trace = debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS, 8);`
`64`	`64`
`65`		`- $file = $trace[1]['file'];`
`66`		`- $line = $trace[1]['line'];`
	`65`	`+ $file = $trace[1]['file'] ?? null;`
	`66`	`+ $line = $trace[1]['line'] ?? null;`
`67`	`67`
`68`	`68`	`$handleTraitFile = (new \ReflectionClass(HandleTrait::class))->getFileName();`
`69`	`69`	`$found = false;`
`@@ -97,9 +97,12 @@ private function getCaller(): array`
`97`	`97`	`}`
`98`	`98`	`}`
`99`	`99`
`100`		`- $name = str_replace('\\', '/', $file);`
`101`		`- $name = substr($name, strrpos($name, '/') + 1);`
	`100`	`+ $name = str_replace('\\', '/', (string) $file);`
`102`	`101`
`103`		`- return compact('name', 'file', 'line');`
	`102`	`+ return [`
	`103`	`+ 'name' => substr($name, strrpos($name, '/') + 1),`
	`104`	`+ 'file' => $file,`
	`105`	`+ 'line' => $line,`
	`106`	`+ ];`
`104`	`107`	`}`
`105`	`108`	`}`