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

Silent error when an email template is not found #8437

Closed
shavounet opened this issue Feb 6, 2017 · 8 comments
Closed

Silent error when an email template is not found #8437

shavounet opened this issue Feb 6, 2017 · 8 comments
Assignees
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@shavounet
Copy link

shavounet commented Feb 6, 2017

Preconditions

  1. Tested on Magento CE 2.1.3 with PHP 7

Steps to reproduce

  1. Create a custom controller, sending a custom mail, mostly like this
  2. Make a mistake in the template file name
  3. Try to send the mail

Expected result

  1. The mail is correctly sent with content

Actual result

  1. The mail is correctly sent but the body is empty

IMO, \Magento\Email\Model\AbstractTemplate::loadDefault should throw an error (or at least log an error message...), when the file is not found. In this case $rootDirectory->getRelativePath($templateFile) returns an empty string and so does $rootDirectory->readFile('')... Maybe the problem is deeper, but a silent error should not happen.

If it matters, my send code in the controller was followed by a redirect (after filling a form).

Best way to reproduce

  1. Go to file app/code/Magento/Customer/etc/email_templates.xml
  2. Make a mistake in the template file name: replace from account_new.html to account_new1.html in tag:
...
    <template id="customer_create_account_email_template" label="New Account" file="account_new1.html" type="html" module="Magento_Customer" area="frontend"/>
...
  1. Create new customer on frontend

Expected result

  1. The email about new customer not sent.
  2. Exception like "Can not find the file account_new1.htm" is present in logs

Actual result

  1. The mail is correctly sent but the body is empty
  2. Any exception is absent in logs.
@vrann
Copy link
Contributor

vrann commented Mar 14, 2017

@shavounet I see the following code in the logic of the filesystem adapter which should throw an exception in case if you misspelled the name of the file:
https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/Filesystem/Driver/File.php#L149
Do you see why doesn't it throw it?
Can you provide the code snippet of your controller sending email, please?

@shavounet
Copy link
Author

I don't have much time to test it again, but here is the main method.

    /**
     * Send the quote email
     * @param Quote $quote
     * @throws \Exception
     */
    public function sendNewQuoteEmail($quote)
    {
        $this->inlineTranslation->suspend();

        $receiverEmail = $this->scopeConfig->getValue('vendor_personalization/quote/contact_email', ScopeInterface::SCOPE_STORES);

        if(empty($receiverEmail)) {
            throw new \Exception('The receiver email is undefined in Vendor configuration');
        }

        $this->transportBuilder
            // Set required data
            ->setTemplateIdentifier('vendor_pms_new_quote')
            ->setTemplateOptions([
                'area' => \Magento\Framework\App\Area::AREA_FRONTEND,
                'store' => $this->storeManager->getStore()->getId()
            ])
            ->setTemplateVars(['quote' => $quote])
            ->setFrom([
                'name' => $quote->getFullName(),
                'email' => $quote->getEmail(),
            ])
            ->addTo($receiverEmail)

            // Send message
            ->getTransport()
            ->sendMessage();

        $this->inlineTranslation->resume();
    }

@vrann
Copy link
Contributor

vrann commented Mar 14, 2017

@shavounet thank you let me try

@magento-engcom-team magento-engcom-team added G1 Passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team
Copy link
Contributor

@shavounet , thank you for your report.
Unfortunately, link from provided steps to reproduce returns 404.
We were not able to reproduce this issue. Please provide more detailed steps to reproduce or try to reproduce this issue on a clean installation or latest release.

@magento-engcom-team magento-engcom-team added Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Progress: needs update labels Oct 12, 2017
@magento-engcom-team magento-engcom-team self-assigned this Oct 12, 2017
@shavounet
Copy link
Author

Hello @magento-engcom-team,
The implementation details are provided in my 2nd comment, it's a very simple implementation. I don't have a test environment anymore to provide more feedback, but did you try it out ? I have to admit I agree with @vrann ; it shouldn't fail... yet it did

Maybe there is some side effect with other methods ? (getScheme, getAbsolutePath, ...) If you look at those examples, some case (2nd and 3rd) can be problematic :

php > $a = @file_get_contents('');
php > var_dump($a);
bool(false)
php > $a = @file_get_contents('/');
php > var_dump($a);
string(0) ""
php > $a = @file_get_contents('file://');
php > var_dump($a);
string(0) ""

@magento-engcom-team
Copy link
Contributor

Update description:

Preconditions

  1. Tested on Magento CE 2.1.3 with PHP 7

Steps to reproduce

  1. Create a custom controller, sending a custom mail, mostly like this
  2. Make a mistake in the template file name
  3. Try to send the mail

Expected result

  1. The mail is correctly sent with content

Actual result

  1. The mail is correctly sent but the body is empty

IMO, \Magento\Email\Model\AbstractTemplate::loadDefault should throw an error (or at least log an error message...), when the file is not found. In this case $rootDirectory->getRelativePath($templateFile) returns an empty string and so does $rootDirectory->readFile('')... Maybe the problem is deeper, but a silent error should not happen.

If it matters, my send code in the controller was followed by a redirect (after filling a form).

Best way to reproduce

  1. Go to file app/code/Magento/Customer/etc/email_templates.xml
  2. Make a mistake in the template file name: replace from account_new.html to account_new1.html in tag:
...
    <template id="customer_create_account_email_template" label="New Account" file="account_new1.html" type="html" module="Magento_Customer" area="frontend"/>
...
  1. Create new customer on frontend

Expected result

  1. The email about new customer not sent.
  2. Exception like "Can not find the file account_new1.htm" is present in logs

Actual result

  1. The mail is correctly sent but the body is empty
  2. Any exception is absent in logs.

@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Nov 10, 2017
@magento-engcom-team
Copy link
Contributor

@shavounet, thank you for your report.
We've created internal ticket(s) MAGETWO-83674 to track progress on the issue.

@magento-engcom-team magento-engcom-team added Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 10, 2017
@RomaKis RomaKis self-assigned this Dec 6, 2017
@okorshenko
Copy link
Contributor

Hi @shavounet
The issue has been fixed and delivered in 2.2-develop branch. Will be available with upcoming patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

5 participants