webfactory / WebfactoryPolyglotBundle

Symfony bundle simplifying translations in Doctrine.
MIT License
3 stars 2 forks source link

If the translation table of an entity does not exist, a reasonable error message has to be produced #1

Closed dword-design closed 8 years ago

dword-design commented 8 years ago

If the translation table of an entity does not exist (for example a class has a camel case name and the table uses the underscore notation and the @Table annotation is missing), a cryptical error message is produced:

ERROR: METHOD WEBFACTORY\BUNDLE\POLYGLOTBUNDLE\DOCTRINE\MANAGEDTRANSLATIONPROXY::__TOSTRING() MUST NOT THROW AN EXCEPTION

The source of the error is \Doctrine\ORM\Persisters\Entity\BasicEntityPersister::loadCriteria() where the database query fails. I guess the check for the existence of the table should be on a higher level.

MalteWunsch commented 8 years ago

I tried reproducing your problem, but I already get a helpful error message like this:

An exception has been thrown during the rendering of a template ("An exception occurred while executing 'SELECT t0.nav AS nav1, t0.title AS title2, t0.text AS text3, t0.id AS id4, t0.locale AS locale5, t0.entity_id AS entity_id6 FROM DocumentTranslation t0 WHERE (t0.locale = ? AND t0.entity_id = ?)' with params ["en", 1119]:

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'youthreporter.DocumentTranslation' doesn't exist") in src/Youthreporter/Bundle/WebsiteBundle/Resources/views/Layout/base.html.twig at line 124.

Can you provide more information on how to reproduce?

dword-design commented 8 years ago

You can reproduce it in youthreporter on the route .../en/hinweis/yr locally by explicitly setting a non-existent translation table in HinweisTranslation. I haven't figured out yet why it works for documents. Anyway, the problem seems to be that __toString is not supposed to throw any exceptions.

MalteWunsch commented 8 years ago

It's a bit of a messy situation. On the one hand, we probably don't want to check the existence of the translation table too early, e.g. during the construction of the ManagedTranslationProxy, as this would add a database request that might not ne necessary at all, contradicting the lazy load approach.

On the other hand, it's a PHP limitation that we cannot throw exceptions during __toString(). That limitation ate up the helpful exception Doctrine has thrown in your case.

An idea I'm not convinced of, just for brainstorming: We could catch the exception and use it's error message as the translation. That could catch a developer's eye when they are reading a page. Wouldn't help with automated tests, though. Using the error message that way should only be done in debug environments, as we don't want to expose internals to the public, and should cause an error to be logged.

Better ideas?