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

"From Header set twice" when sending an email #7739

Closed
RishabhRkRai opened this issue Dec 9, 2016 · 27 comments
Closed

"From Header set twice" when sending an email #7739

RishabhRkRai opened this issue Dec 9, 2016 · 27 comments
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed 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.2.x The issue has been reproduced on latest 2.2 release

Comments

@RishabhRkRai
Copy link

RishabhRkRai commented Dec 9, 2016

Preconditions

  1. I use v2.1.1 of Magento and MySQL 5.6.x
  2. I make a custom module which send an email to person more like subscription, when new varients of an item is come.

Steps to reproduce

  1. Make a custom module, which store email and product id in a custom database.
  2. Whenever something new new book edition of that product id will came. It checks for that custom table and send email to that saved email id.
  3. I use Magento "TransportBuilder" to send mails.

Expected result

  1. Emails have been send to all the saved mails in custom table without any error.

Actual result

  1. My category page have no product for the product id belongs to.
  2. Email for the first data found send but for the rest it gives an error in which throw in exception.log.
  3. There is an log in exception.log
    main.CRITICAL: exception 'Zend_Mail_Exception' with message 'From Header set twice' in /../../../../vendor/magento/zendframework1/library/Zend/Mail.php:680
@RishabhRkRai
Copy link
Author

Any update on this.

@magento-engcom-team magento-engcom-team added 2.1.x bug report Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed labels Sep 11, 2017
@toshz
Copy link

toshz commented Sep 29, 2017

Same issue here. Trying to send two emails. One for order confirmation and the other is for a specific type of product from the same order. I get this error:
==> var/log/system.log <==
[2017-09-29 11:33:11] main.ERROR: From Header set twice [] []

Magento CE 2.1.5
PHP 5.6

@mahesh-rajawat
Copy link
Member

mahesh-rajawat commented Oct 26, 2017

@toshz if you need to fix in your custom module then you can read this blog for the solution.
https://webkul.com/blog/magento2-email-header-set-twice-error-custom-module/

@magento-engcom-team
Copy link
Contributor

@RishabhRkRai, thank you for your report.
We were not able to reproduce this issue by following the steps you provided. If you'd like to update it, please reopen the issue.
We tested the issue on 2.1.9

@magento-engcom-team magento-engcom-team added the Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch label Nov 6, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Nov 7, 2017
@tomlutzenberger
Copy link

tomlutzenberger commented Feb 5, 2018

Had the same issue sending only 1 mail after a new customer registered.
The problem was, that I created a new transportBuilder using the ObjectManager instead of DI.
Don't do that
Make use of proper dependency injection.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Jul 24, 2018

It seems to me that it's a known issue, but it's hard to reproduce it.

Looks like this issue duplicating #14952.

Currently we have Pull Request #16461 that should fix this issue, but it's not accepted because it's not clear how to fix this issue in correct way.

@stephan-cream
Copy link
Contributor

stephan-cream commented Aug 22, 2018

I found out how to reproduce this.
When sending multiple mails in the same process.
A cronjob would be a good example.
Another good example would be a mass action like the one provided by Mageworx's module-ordersgrid module to resend order emails, invoice emails and shipment emails.

The main cause is that a singleton is being used in the following files:

  • vendor/magento/framework/Mail/Template/TransportBuilder.php
  • vendor/magento/framework/Mail/Template/TransportBuilderByStore.php
    This singleton is the message singleton (an instance of Magento\Framework\Mail\MessageInterface).
    Zend_Mail gives an error when the header is set twice, subject, date, recipients, etc.
    Since a singleton is being used, once one of them is sent, during the next mail attempt it will try to configure the above headers and will give an error message.
    This is because the singleton instance is never properly 'cleared/reset'.

There is a reset function that is called on the TransportBuilder but a simple mistake is made here.
Instead of clearing the message singleton's headers / variables it is created anew with the following code: $this->message = $this->objectManager->create(\Magento\Framework\Mail\Message::class);.
However the singleton that was loaded in TransportBuilderByStore is not reset with this, only the one within TransportBuilder.
As such the from header and variable is still set.
I believe you should replace the above code with the following.
$this->message->clearDate();
$this->message->clearFrom();
$this->message->clearMessageId();
$this->message->clearRecipients();
$this->message->clearReplyTo();
$this->message->clearReturnPath();
$this->message->clearSubject();

This fix works for me at least.
If i am missing something please let me know.

@stephan-cream stephan-cream reopened this Aug 22, 2018
@ihor-sviziev ihor-sviziev added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release and removed bug report labels Aug 22, 2018
@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Aug 22, 2018

Hi @stephan-cream as I said before - this issue duplicating #14952.
We have potential fix in #16461 (but as it's not merged yet - we might change implementation).

I just updated labels and I'll keet this issue open

@ihor-sviziev ihor-sviziev added bug report and removed Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch bug report labels Aug 22, 2018
@ghost ghost self-assigned this Aug 30, 2018
@ghost
Copy link

ghost commented Aug 30, 2018

Duplicate of #14952

@ghost ghost marked this as a duplicate of #14952 Aug 30, 2018
@ghost ghost closed this as completed Aug 30, 2018
@jaykobi
Copy link

jaykobi commented Aug 30, 2018

I have the same problem but the solution from @stephan-cream did not work for me. To make magento send my invoice mails again, until the official fix is published, I changed vendor\magento\framework\Mail\Template\TransportBuilderByStore

public function setFromByStore($from, $store)
    {
        $result = $this->senderResolver->resolve($from, $store);
        $this->message->setFrom($result['email'], $result['name']);

        return $this;
    }

to

    public function setFromByStore($from, $store)
    {
        $result = $this->senderResolver->resolve($from, $store);

        if ($this->message->getFrom()) {
            $this->message->clearFrom();
        }

        $this->message->setFrom($result['email'], $result['name']);

        return $this;
    }

@loekderooij
Copy link

loekderooij commented Sep 17, 2018

It is easier to define the injected transport object as non singleton. You can do this by defining your injected object in di.xml and setting the attribute 'shared' to 'false'. For instance, I created a script where I needed to send multiple order mails. Adding the following resolve my problem:

<type name="\Magento\Sales\Model\Order\Email\SenderBuilder">
        <arguments>
            <argument name="transportBuilder" xsi:type="object" shared="false">Magento\Framework\Mail\Template\TransportBuilder</argument>
            <argument name="transportBuilderByStore" xsi:type="object" shared="false">Magento\Framework\Mail\Template\TransportBuilderByStore</argument>
        </arguments>
    </type>

@ihor-sviziev
Copy link
Contributor

@LuckyLoek unfortunately it will not be enough because they both contains "message" object that should be shared.

@gwharton
Copy link
Contributor

Fixed in 2.2-develop
9a7c9e5#diff-dfb3ee5a7f863458afea75f5524cd2e5

@hostep
Copy link
Contributor

hostep commented Dec 21, 2018

@gwharton: just ran into this problem on Magento 2.2.5 shop (without amazon modules), and can not confirm that 9a7c9e5 fixes the "From Header set twice" problem. In fact, it makes the email system even more unstable, by causing the From email address to be incorrect.
Also when async sending of emails is enabled, the order emails weren't getting marked as sent anymore, so they got send every single minute again by the cronjob in our case.

By then applying your fix #18472 as well on top of this, we get a much more stable email system, no more "From Header set twice" problems, also no incorrect From email addresses and async sending now marks emails as being sent.

Just wanted to add this extra info in here in case anybody else ran against this.

Big thanks for the fix!

@magento-engcom-team magento-engcom-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Jan 7, 2019
@stephan-cream
Copy link
Contributor

@magento-engcom-team give me 2.2.7-develop instance

@magento-engcom-team
Copy link
Contributor

Hi @stephan-cream. Thank you for your request. I'm working on Magento 2.2.7-develop instance for you

@gwharton
Copy link
Contributor

@stephan-cream I think you will find this is still broken in 2.2.7.

I'm hoping that #18472 will be merged soon in time for 2.2.8 which resolves this issue.

@stephan-cream
Copy link
Contributor

I know, hence i was trying to get a clean install to make a screencapture.
It is broken because of vendor/magento/framework/Mail/Template/TransportBuilder.php.
Since in the public function reset() the message singleton is replaced with a new instance (non-singleton) the From email/name which is set in vendor/magento/framework/Mail/Template/TransportBuilderByStore.php will not be available in this new non-singleton which is eventually used to send the email.
Like i previously stated, replacing a singleton with a new instance won't fix this issue, in fact now emails can't be sent at all because there is no 'From' available.

@gwharton
Copy link
Contributor

Yeah, If you apply #18472, it gets rid of the use of TransportBuilderByStore altogether.

@crankycyclops
Copy link
Contributor

@gwharton, I'm really confused, because I'm looking at my 2.2.7 instance (installed via composer), and when I go through the files in the diff manually, I see that all the changes in #18472 seem to have already been merged. Yet I still get the "From Header sent twice" error.

@gwharton
Copy link
Contributor

gwharton commented Mar 4, 2019

Yes. Im also confused. The source code for 2.2.7 on github does NOT have 18472 merged. I just checked.

@crankycyclops
Copy link
Contributor

crankycyclops commented Mar 4, 2019

@gwharton, that is indeed very strange. I also went back to browse the files in the 2.2.7 Github release, and I can see that the changes weren't applied like you said. Yet they are in the 2.2.7 version I installed via composer.

At any rate, after verifying that those changes are a part of my code, I still get the "From Header set twice" error, so it looks like I'll have to dig into Magento core some more to see what's up. Like others, this bug seems to occur for me when I try to send more than one email in the same process.

EDIT: Some digging reveals that the issue I'm having might be due to a bug in my own module's code. That being said, @skymeissner's solution (#7739 (comment)) seemed to make my mail system more robust. Whereas the bug in my code prevented all emails in the process from being sent out, applying his fix allowed at least the emails not related to my own code to go out. Thought that might be worth mentioning, as it may be worth rolling it into the official Magento core. For now, I'm just going to maintain my own patch for it that I'll apply whenever I update via composer.

@gwharton
Copy link
Contributor

gwharton commented Mar 5, 2019

Just be aware that when you get the error saying From set twice, this means that the $message object that Magento is using to formulate an email has already been used for a previous email and still has content in it. In the #7739 fix, the from address is cleared before being reset, but what about the other fields. Sure the body and subject will be rewritten as the new email is formed, but what if for example the previous user of the $message object had a Bcc or cc set. Will this be cleared also? Could you inadvertently release sensitive info. This may be the preferred behaviour when the second email is a copy of the first, for example to admin, but what if the second email is unrelated to the first.

There is some logic inside the Transport Builder class in the getTransport and reset functions where the "reusing" of message objects happens. Would be worth checking that out and making sure its doing exactly what you think it is doing.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Mar 11, 2019

FYI This issue was fixed in 2.2-develop branch and planned to be included to 2.2.9 release
#18472

@ihor-sviziev ihor-sviziev added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Mar 11, 2019
@tuyennn
Copy link
Member

tuyennn commented Jul 17, 2019

@gwharton Is there anyway to approach the problem in another way implementation while sending multiple emails because seem message still used from subject etc from previous mail?

@gwharton
Copy link
Contributor

Someone from Magento will need to pick this up and look at what needs to be done moving forward.

This issue was closed.
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 Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed 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.2.x The issue has been reproduced on latest 2.2 release
Projects
None yet
Development

No branches or pull requests