FlattenException incompatibility with Sf2.3

[odino]

Hey guys, not sure if this is the best approach, as it requires dropping backwards compatibility.

Basically in Symfony2.3, afaik, they introduce a new debug component. The FlattenException has been moved there, and for BC what they did is to keep the one in HttpFoundation, by making it extend the one in Debug.

That's cool but then this Exception controller intercepts internal exceptions from Sf2, and the Exceptions thrown are the ones from the Debug component. Therefore you get amazing things like:

PHPUnit_Framework_Error: Argument 2 passed to FOS\RestBundle\Controller\ExceptionController::showAction() must be an instance of Symfony\Component\HttpKernel\Exception\FlattenException, instance of Symfony\Component\Debug\Exception\FlattenException given)

I just noticed it after updating our libs via composer. Am I missing something?

Dunno your thoughts about this, just suggesting to find a quick fix :)

[odino]

Have to add that the problem is not there on 2.3.4, once I update to 2.3.5 everything explodes :)

[odino]

git diff v2.3.5 v2.3.4 shows it pretty clearly:

diff --git a/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php b/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php
index 3abc2a9..5a52791 100644
--- a/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php
+++ b/src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php
@@ -12,7 +12,7 @@
 namespace Symfony\Bundle\TwigBundle\Controller;

 use Symfony\Bundle\FrameworkBundle\Templating\TemplateReference;
-use Symfony\Component\Debug\Exception\FlattenException;
+use Symfony\Component\HttpKernel\Exception\FlattenException;

[odino]

@liuggio points to this

[odino]

Not sure what they will be doing in the symfony/symfony repo, I think it's better to wait for them to take a decision

[Tobion]

This should be merged in any case. Independent of the decision in symfony.
But you need to raise the dependency on framework-bundle in composer.json to ~2.3.

[stof]

To ~2.4 as the BC rbeak just got reverted in 2.3

[Tobion]

Still the dependency is ~2.3 as the class was introduced in 2.3 and it should also work in 2.3. Or not?

[stof]

Ah sorry, the exception controller is not extending the TwigBundle one anyway, so typehint the new class would indeed work

[lsmith77]

I would prefer dropping the type hint and then checking manually if its one of the possible FlattenException instances so that we do not need to maintain different branches just for this.

[lsmith77]

btw .. while you are at it .. could you also look into this change here https://github.com/symfony/symfony/pull/9155/files and if its relevant to this controller too?

[odino]

@lsmith77 agree that dropping type hinting is not ideal but probably the most viable solution here, will come up with something in a bit

[Tobion]

@lsmith77 why do you need different branches? I don't see the need. The dependencies got raised but that does not require a new branch. Unless you really want to have a maintenance branch for symfony < 2.3. But symfony 2.2 is only maintained itself for 2 more month.

[lsmith77]

I would like to maintain BC unless there is a really good reason to move on. This doesn't seem like a good reason to move on.

[stof]

@Tobion As 2.2 is still maintained, we wtill need to maintain a version of the bundle compatible with it

[Tobion]

@lsmith77 it's not possible to maintain bc here. That's the same problem as in symfony/symfony#9145. Removing the typehint is also a bc break for people extending the class/method.

[lsmith77]

I can live with this BC break as we are not yet at 1.0.0 stable. its an easy enough fix :)

[Tobion]

@stof so all FOS bundles have the same maintenance process as symfony itself? That's probably pretty hard to achieve considering there a less people involved to maintain all the bundles than for symfony. And it should probably be documented that there is version of each bundle maintained for each symfony minor version.

[stof]

@Tobion No, we try as much as possible to have 1 version of the bundle compatible with all Symfony versions (it is possible most of the time now that 2.0 does not need to be supported anymore)

[odino]

What about all the reference to FlattenException in the phpdoc? Change to mixed?

[lsmith77]

i guess use the fcqn and separate with |

[Tobion]

@stof that means yes, you have a version of each bundle maintained for each symfony minor version. In this case it's only one branch. But I questioned when you said that symfony 2.2 is still maintained, we also need to maintain the rest bundle. This means we cannot just drop symfony 2.2 support for fos rest bundle from now on. Although to me we could do so and follow our own maintenance process because this is an independent "organisation".

[lsmith77]

generally the effort isn't too big and the compromises not that significant so yes its usually possible to support all currently maintained symfony versions with one branch. aside from this, i would also only break BC with a non maintained version for a good reason.

[stof]

@Tobion Well, if we were already having a stable release of the bundle, I would be fine with bumping the requirement in the dev version as we would still have a version compatible with 2.2 (the stable one). but we would still need to provide patch releases based on this stable version until the EOL of Symfony 2.2, otherwise we would simply be ignoring some of our users (by forcing them to upgrade Symfony to get a maintained version of the bundle). I don't think we can afford ignoring users of maintained Symfony version for bundles, as bundles are plugins for Symfony. It would not be a good behavior IMO.

However, I don't understand what you mean with "our own maintenance process".
We follow our own release schedule based on the changes done in the bundle, and we don't try to synchronize releases with Symfony ones when it does not make sense for the bundle.

[odino]

let me know guys

[lsmith77]

looks good to me.

[Tobion]

I would inline this code. It's only called once and should not be protected anyway.

[lsmith77]

agreed

[odino]

@lsmith77 done

[lsmith77]

ah one last thing .. can you add a note about the BC break into the UPGRADING.md?

[odino]

from 0.13.1 or what?

[lsmith77]

yes

[odino]

done, will you also tag?

[lsmith77]

see #566

[iagomez]

Guys,

I just performed these updates:
Symfony 2.3.4 -> 2.3.5
FOSRestBundle 0.12.0 -> 0.13.1

My Exceptions are no longer being processed by FOS\RestBundle\Controller\ExceptionController::showAction. Instead of getting a json object with the Status Code and the Exception Message I get the html response:
HTTP/1.0 500 Internal Server Error
Date: Tue, 01 Oct 2013 17:00:20 GMT
Server: Apache/2.4.4 (Fedora) OpenSSL/1.0.1e-fips PHP/5.5.1
X-Powered-By: PHP/5.5.1
Cache-Control: max-age=604800
Expires: Tue, 08 Oct 2013 17:00:20 GMT
Content-Length: 0
Connection: close
Content-Type: text/html; charset=UT

I stepped through with the debugger to confirm that my controller is in fact throwing the exception

My config.yml contains
twig:
debug: %kernel.debug%
strict_variables: %kernel.debug%
exception_controller: 'FOS\RestBundle\Controller\ExceptionController::showAction'

Is this related to this issue? Any advice?

[iagomez]

I downgraded back to Symfony 2.3.4 and FOS\RestBundle\Controller\ExceptionController is handling my exceptions again.

Let me know if you need me to provide information about my system.

[stof]

@iagomez Yes it is related. As this has been merged, you can update FOSRestBundle to its dev version and it should work with 2.3.5

[iagomez]

@stof Thank you.