E576 bug #17744 Improve error reporting in router panel of web profiler (j… · symfony/symfony@9d58ad9 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9d58ad9

Browse files
fabpotfabpot
committed
bug #17744 Improve error reporting in router panel of web profiler (javiereguiluz)
This PR was squashed before being merged into the 2.7 branch (closes #17744). Discussion ---------- Improve error reporting in router panel of web profiler | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17342 | License | MIT | Doc PR | - ### Problem If you define a route condition like this: ```yaml app: resource: '@AppBundle/Controller/' type: annotation condition: "request.server.get('HTTP_HOST') matches '/.*\.dev/'" ``` When browsing the Routing panel in the web profiler, you see an exception: ![problem](https://cloud.githubusercontent.com/assets/73419/12930027/553eeb08-cf76-11e5-90b1-ab0de6175d4e.png) #### Why? Because the route condition uses the `request` object, but the special `TraceableUrlMatcher` class doesn't get access to the real `request` object but to the special object obtained via: ```php $request = $profile->getCollector('request'); ``` These are the contents of this pseudo-request: ![cause](https://cloud.githubusercontent.com/assets/73419/12930052/804ea248-cf76-11e5-9c38-2e43e1654065.png) `request.server.get(...)` condition fails because `request.server` is `null`. The full exception message shows this: ![exception](https://cloud.githubusercontent.com/assets/73419/12930079/9c7d6058-cf76-11e5-8eeb-45f5059c824c.png) ### Solution I propose to catch all exceptions in `TraceableUrlMatcher` and display an error message with some details of the exception: ![error_message](https://cloud.githubusercontent.com/assets/73419/12930106/b29e31d2-cf76-11e5-868c-98d8b0cc4e5b.png) Commits ------- 1001554 Improve error reporting in router panel of web profiler
2 parents 46d1d24 + 1001554 commit 9d58ad9

File tree

3 files changed

+59
-5
lines changed

3 files changed

+59
-5
lines changed

‎src/Symfony/Bundle/WebProfilerBundle/Controller/RouterController.php‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\Routing\RouterInterface;
1919
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
2020
use Symfony\Component\HttpKernel\Profiler\Profiler;
21+
use Symfony\Component\HttpKernel\DataCollector\RequestDataCollector;
2122

2223
/**
2324
* RouterController.
@@ -62,16 +63,39 @@ public function panelAction($token)
6263

6364
$profile = $this->profiler->loadProfile($token);
6465

65-
$context = $this->matcher->getContext();
66-
$context->setMethod($profile->getMethod());
67-
$matcher = new TraceableUrlMatcher($this->routes, $context);
68-
66+
/** @var RequestDataCollector $request */
6967
$request = $profile->getCollector('request');
7068

7169
return new Response($this->twig->render('@WebProfiler/Router/panel.html.twig', array(
7270
'request' => $request,
7371
'router' => $profile->getCollector('router'),
74-
'traces' => $matcher->getTraces($request->getPathInfo()),
72+
'traces' => $this->getTraces($request, $profile->getMethod()),
7573
)), 200, array('Content-Type' => 'text/html'));
7674
}
75+
76+
/**
77+
* Returns the routing traces associated to the given request.
78+
*
79+
* @param RequestDataCollector $request
80+
* @param string $method
81+
*
82+
* @return array
83+
*/
84+
private function getTraces(RequestDataCollector $request, $method)
85+
{
86+
$traceRequest = Request::create(
87+
$request->getPathInfo(),
88+
$request->getRequestServer()->get('REQUEST_METHOD'),
89+
$request->getRequestAttributes()->all(),
90+
$request->getRequestCookies()->all(),
91+
array(),
92+
$request->getRequestServer()->all()
93+
);
94+
95+
$context = $this->matcher->getContext();
96+
$context->setMethod($method);
97+
$matcher = new TraceableUrlMatcher($this->routes, $context);
98+
99+
return $matcher->getTracesForRequest($traceRequest);
100+
}
77101
}

‎src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Routing\Matcher;
1313

14+
use Symfony\Component\HttpFoundation\Request;
1415
use Symfony\Component\Routing\Exception\ExceptionInterface;
1516
use Symfony\Component\Routing\Route;
1617
use Symfony\Component\Routing\RouteCollection;
@@ -40,6 +41,15 @@ public function getTraces($pathinfo)
4041
return $this->traces;
4142
}
4243

44+
public function getTracesForRequest(Request $request)
45+
{
46+
$this->request = $request;
47+
$traces = $this->getTraces($request->getPathInfo());
48+
$this->request = null;
49+
50+
return $traces;
51+
}
52+
4353
protected function matchCollection($pathinfo, RouteCollection $routes)
4454
{
4555
foreach ($routes as $name => $route) {

‎src/Symfony/Component/Routing/Tests/Matcher/TraceableUrlMatcherTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Routing\Tests\Matcher;
1313

14+
use Symfony\Component\HttpFoundation\Request;
1415
use Symfony\Component\Routing\Route;
1516
use Symfony\Component\Routing\RouteCollection;
1617
use Symfony\Component\Routing\RequestContext;
@@ -98,4 +99,23 @@ public function getLevels($traces)
9899

99100
return $levels;
100101
}
102+
103+
public function testRoutesWithConditions()
104+
{
105+
$routes = new RouteCollection();
106+
$routes->add('foo', new Route('/foo', array(), array(), array(), 'baz', array(), array(), "request.headers.get('User-Agent') matches '/firefox/i'"));
107+
108+
$context = new RequestContext();
109+
$context->setHost('baz');
110+
111+
$matcher = new TraceableUrlMatcher($routes, $context);
112+
113+
$notMatchingRequest = Request::create('/foo', 'GET');
114+
$traces = $matcher->getTracesForRequest($notMatchingRequest);
115+
$this->assertEquals("Condition \"request.headers.get('User-Agent') matches '/firefox/i'\" does not evaluate to \"true\"", $traces[0]['log']);
116+
117+
$matchingRequest = Request::create('/foo', 'GET', array(), array(), array(), array('HTTP_USER_AGENT' => 'Firefox'));
118+
$traces = $matcher->getTracesForRequest($matchingRequest);
119+
$this->assertEquals('Route matches!', $traces[0]['log']);
120+
}
101121
}

0 commit comments

Comments
 (0)
0