Skip to content

Commit 1c61b16

Browse files
authored
fix Pre Hook Exception in case _controller is not stringable (open-telemetry#366)
* fix Pre Hook Exception in case _controller is not stringable * add new test_http_kernel_handle_subrequest_with_various_controller_types integation test
1 parent 8724759 commit 1c61b16

File tree

2 files changed

+55
-1
lines changed

2 files changed

+55
-1
lines changed

src/Instrumentation/Symfony/src/SymfonyInstrumentation.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,14 @@ public static function register(): void
4747
$request = ($params[0] instanceof Request) ? $params[0] : null;
4848
$type = $params[1] ?? HttpKernelInterface::MAIN_REQUEST;
4949
$method = $request?->getMethod() ?? 'unknown';
50+
$controller = $request?->attributes?->get('_controller');
51+
52+
if (!is_callable($controller, true, $controllerName)) {
53+
$controllerName = 'sub-request';
54+
}
55+
5056
$name = ($type === HttpKernelInterface::SUB_REQUEST)
51-
? sprintf('%s %s', $method, $request?->attributes?->get('_controller') ?? 'sub-request')
57+
? sprintf('%s %s', $method, $controllerName)
5258
: $method;
5359
/** @psalm-suppress ArgumentTypeCoercion */
5460
$builder = $instrumentation

src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,54 @@ public function test_http_kernel_handle_subrequest(): void
204204
$this->assertSame(SpanKind::KIND_INTERNAL, $span->getKind());
205205
}
206206

207+
public function test_http_kernel_handle_subrequest_with_various_controller_types(): void
208+
{
209+
$kernel = $this->getHttpKernel(new EventDispatcher());
210+
211+
// String controller
212+
$request = new Request();
213+
$request->attributes->set('_controller', 'SomeController::index');
214+
$kernel->handle($request, \Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST);
215+
$this->assertSame('GET SomeController::index', $this->storage[0]->getName());
216+
$this->storage->exchangeArray([]);
217+
218+
// Array: [object, method]
219+
$controllerObj = new class() {};
220+
$request = new Request();
221+
$request->attributes->set('_controller', [$controllerObj, 'fooAction']);
222+
$kernel->handle($request, \Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST);
223+
$this->assertSame('GET ' . get_class($controllerObj) . '::fooAction', $this->storage[0]->getName());
224+
$this->storage->exchangeArray([]);
225+
226+
// Array: [class, method]
227+
$request = new Request();
228+
$request->attributes->set('_controller', ['SomeClass', 'barAction']);
229+
$kernel->handle($request, \Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST);
230+
$this->assertSame('GET SomeClass::barAction', $this->storage[0]->getName());
231+
$this->storage->exchangeArray([]);
232+
}
233+
234+
/**
235+
* @psalm-suppress UnevaluatedCode
236+
*/
237+
public function test_http_kernel_handle_subrequest_with_null_and_object_controller(): void
238+
{
239+
$kernel = $this->getHttpKernel(new EventDispatcher());
240+
241+
// Object controller (should fallback to 'sub-request')
242+
$controllerObj2 = new class() {};
243+
$request = new Request();
244+
$request->attributes->set('_controller', $controllerObj2);
245+
$kernel->handle($request, \Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST);
246+
$this->assertSame('GET sub-request', $this->storage[0]->getName());
247+
248+
// Null/other controller (should fallback to 'sub-request')
249+
$request = new Request();
250+
$request->attributes->set('_controller', null);
251+
$kernel->handle($request, \Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST);
252+
$this->assertSame('GET sub-request', $this->storage[0]->getName());
253+
}
254+
207255
private function getHttpKernel(EventDispatcherInterface $eventDispatcher, $controller = null, ?RequestStack $requestStack = null, array $arguments = []): HttpKernel
208256
{
209257
$controller ??= fn () => new Response('Hello');

0 commit comments

Comments
 (0)