[Routing] Properly handle Responses and exceptions by displaying 404 pages, etc.

[cmfcmf]

• handle responses
• handle exceptions
• handle legacy mode
• handle Twig responses
• handle new Controller
• see http://zikula.org/index.php/forums/topic/59739

refs #1552

[craigh]

handle exceptions

Symfony does some of this for us:

By default, the showAction() method of the ExceptionController will be called when an exception occurs.

This controller will either display an exception or error page, depending on the setting of the kernel.debug flag. While exception pages give you a lot of helpful information during development, error pages are meant to be shown to the user in production.

http://symfony.com/doc/current/cookbook/controller/error_pages.html

the debug flag is set to false in most situations I've seen (including zikula.org I think) so we are still seeing all exceptions all the time.

[craigh]

also, all twig stuff is moved to 1.5

[craigh]

I've looked through the entire /system directory at only the controllers and found that only the following exceptions are in use:

• AccessDeniedException
• NotFoundHttpException
• InvalidArgumentException
• FatalErrorException
• RuntimeException
• Exception

Of these, AccessDeniedException is now handled. NotFoundHttpException and InvalidArgumentException are used quite often. FatalErrorException is used some and the others are used rarely. What shall we do with the these?

[craigh]

I did not scan any API or other classes in /system and I did not scan other parts of the core. I guess I'm thinking those should be handled differently (or not handled) as they are the responsibility of the developer to handle.

[Guite]

For a NotFoundHttpException or a InvalidArgumentException we could display an error page.
Not sure about the others... at the end we log the error in all cases and show it in the output, right?

Slightly related but for future: http://symfony.com/blog/new-in-symfony-2-6-error-page-previews

[craigh]

[11/17/14, 4:00:46 PM] Craig Heydenburg: can you tell me more about #1789
[11/17/14, 4:00:51 PM] Craig Heydenburg: ?
[11/17/14, 4:03:29 PM] Christian: Sure. Basically, no matter what happens in a controller, the page shouldn't blow up. This includes but is not limited to:

• NotFoundException
• FatalErrorException
• Any Exception
• AccessDeniedException
• RedirectResponses
• Old Zikula_Response_Ajax_* responses
• New Symfony responses

[craigh]

This ticket is mainly about using the new AbstractController and concerns about it's functionality. Especially with regard to theming and differences in how it might send responses, e.g.

return new Response($this->view->fetch('...'));
and 
return new Response("{json-data}")
and 
return new Response($twig->render('bla'))

[craigh]

ping @Kaik can you help out here by telling us how you have tested the new AbstractController and what your experience was?

[Kaik]

That was long time ago I was able to display page with twig and some data from db and actually that was all, no theme wrapped, no translation, no forms, just plain page and items list and that was really long time ago...

[craigh]

I wrote a small module that can be used to test Exceptions and other things we want to break. feel free to expand the scope of the module 😄

https://github.com/craigh/BreakIt

ping @cmfcmf

the bad news is, zikula isn't yet performing as I expected with regard to this ticket.

[craigh]

[craigh]

OK - I don't know if we are ready to close this ticket yet.

Everything that is currently testable by BreakIt 💣 seems to be covered:

When in prod environment:
Throw AccessDeniedException - redirects either to login or to referrer with message
Other exceptions go to an error page that we can customize further if needed, but currently looks like this:

BreakIt also tests an invalid route (/I/AM/INVALID) which at this time simply redirects to the home page

and lastly and old-style invalid route (/index.php?module=invalid&type=user&func=main) which looks like so:

what else should be done?

[craigh]

one interesting point I just learned:

because Core::init() runs before any REQUEST events, the old System::queryStringDecode() fires before listeners like src/lib/Zikula/Bundle/CoreBundle/EventListener/LegacyRouteListener.php but the latter is what is actually handling both invalid urls above.

[craigh]

closing. If other issues are discovered, they should be opened as separate issues.

[craigh]

@Kaik - you might want to check out how I am using Twig in https://github.com/craigh/BreakIt

[cmfcmf]

If other issues are discovered, they should be opened as separate issues.

👍