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.

Sign up for GitHub

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

EmailTemplateCompilationException wrong description message #1054

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

EmailTemplateCompilationException wrong description message #1054

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Oro/Bundle/EmailBundle/Exception/EmailTemplateCompilationException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ class EmailTemplateCompilationException extends EmailTemplateException
*/
public function __construct(EmailTemplateCriteria $criteria)
{
$message = sprintf('Could not found one email template with "%s" name', $criteria->getName());
$message = sprintf(
'Compilation error during rendering template "%s". Please look at the log file for more details.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please look at the log file for more details.

As on a production web server, this message will be already displayed in a log. It's odd to see this recommendation in the exception message. Also, not all applications use files to store logs. And we never recommend the same for any other exception messages, so I propose to remove the second part.

Instead, we can provide more details about an error in the exception itself. For example, now we don't pass the original exception to the parent Exception constructor. While in \Oro\Bundle\EmailBundle\Provider\EmailTemplateContentProvider::getTemplateContent, we do have the original exception, and we can add it to the stack trace, so the developer will not need to check the log file in the dev environment, and the system administrator will see the message in the original trace, that is preferred, instead of having two unrelated log messages, that could even not go in a row, in case of a high load.

$criteria->getName()
);

if ($criteria->getEntityName()) {
$message = sprintf('%s for "%s" entity', $message, $criteria->getEntityName());
Expand Down