Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing _route parameter notice in RouterListener logging case #15392

Closed
wants to merge 1 commit into from

Conversation

Haehnchen
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets na
License MIT
Doc PR na

\Symfony\Component\HttpKernel\EventListener\RouterListener::onKernelRequest missed test cases in logging condition. So if we provide routing matches without a _route parameter notice messages are thrown. In this case i added a check and use n/a for empty route names as this is also shown in profiler if it not exists.

@Tobion
Copy link
Member

Tobion commented Jul 28, 2015

Which third party router do you use that does not set this parameter?

I agree with this PR since the UrlMatcherInterface does not specify that a _route parameter must be returned so it cannot be assumed in this listener.

@xabbuh
Copy link
Member

xabbuh commented Jul 29, 2015

👍

fabpot added a commit that referenced this pull request Jul 29, 2015
…ing case (Haehnchen)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #15392).

Discussion
----------

Fix missing _route parameter notice in RouterListener logging case

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | na
| License       | MIT
| Doc PR        | na

`\Symfony\Component\HttpKernel\EventListener\RouterListener::onKernelRequest` missed test cases in logging condition. So if we provide routing matches without a `_route` parameter `notice` messages are thrown. In this case i added a check and use `n/a` for empty route names as this is also shown in profiler if it not exists.

Commits
-------

0ce91a6 Fix missing _route parameter notice in RouterListener logging case
@fabpot
Copy link
Member

fabpot commented Jul 29, 2015

Merged into 2.3., thanks @Haehnchen

@fabpot fabpot closed this Jul 29, 2015
@Haehnchen
Copy link
Contributor Author

fyi: using cmf routing, but replaced maching for a minimized cache. so no "third party router" issue.

fabpot added a commit to silexphp/Silex that referenced this pull request Sep 10, 2016
This PR was merged into the 2.0.x-dev branch.

Discussion
----------

removed obsolete code (fixed in Symfony 2.3.32)

That was fixed in symfony/symfony#15392 (so before 2.8.0).

Commits
-------

b12cdc4 removed obsolete code (fixed in Symfony 2.3.32)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants