Sending multiple emails using the TransportBuilder class causes an Zend_Mail exception

[david-kominek]

Preconditions

1. Magento 2.2.3
2. PHP 7.0.12
3. MySQL 5.7.18

Steps to reproduce

1. Fresh install of Magento
2. Make a custom module with 2 observer classes on an event('sales_order_place_after'). Each of these observers should inject the TransportBuilder class and send an email:

$transport = $this->transportBuilder
    ->setTemplateIdentifier('email_template_identifier')
    ->setTemplateOptions([
        'area' => \Magento\Framework\App\Area::AREA_FRONTEND,
        'store' => 1
    ])
    ->setTemplateVars($data)
    ->setFrom($sender)
    ->addTo($mailto);

// important that this is its own line for when we override the transport builder due to how PHP polymorphism works :(
$transport = $this->transportBuilder->getTransport();

$transport->sendMessage();

3. Checkout and place an order.

Expected result

1. Two emails will be sent...

Actual result

1. The first email is sent, the second email is never sent...
2. The second observer will throw an exception in the $transport->sendMessage() function: throw new Zend_Mail_Exception('From Header set twice');
3. The system.log file will show the following line: "[2018-03-21 19:32:06] main.ERROR: From Header set twice [] []"

The issue seems to be that the transport builder is not adequately cleaning up the $message variable after getting the transport. The solution is to overwrite the TransportBuilder class, and in the function getTransport() to the following:

<?xml version="1.0" ?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
	<preference for="Magento\Framework\Mail\Template\TransportBuilder" type="Vendor\Module\Model\Mail\TransportBuilder" />
</config>

class TransportBuilder extends \Magento\Framework\Mail\Template\TransportBuilder
{
    public function getTransport()
    {
        $mailTransport = parent::getTransport();
        $this->message->clearFrom()
            ->clearReplyTo()
            ->clearReturnPath()
            ->clearSubject()
            ->clearMessageId()
            ->clearRecipients()
            ->setParts([]);

        return $mailTransport;
    }
}

Edit: I don't feel that @dimonovp really reviewed this ticket. Can we have it reopened and reviewed?

[magento-engcom-team]

@david-kominek, please refer to the Community Forums or the Magento Stack Exchange site for advice or general discussion about this issue. The GitHub issue tracker is intended for Magento Core technical issues only.

[david-kominek]

OK, so I'm just a little confused. The core team intended the TransportBuilder to be a single use class? The way that it currently is implemented, you can only use the TransportBuilder one time per request, no matter which class you inject it into. I thought the point of a "Builder" class was to be able to build multiple of something.

[sebastian-enz]

@magento-engcom-team Are you sure it is appropriate to close this ticket? This seems to be a real issue. TransportBuilder should not be usable once per request only. For example, what about an extension creating 5 shipments for different orders and emailing them to customers, in "one request/execution" -- right now, using Magento\Sales\Model\Order\Email\Sender\ShipmentSender only the first of the five shipment emails would be sent, as after the first email, the above exception described by @david-kominek is stopping further emails from being sent.

[heldchen]

we have this problem too, first noticed after updating to 2.2.4.

the TransportBuilder uses ObjectManager in reset() to create a "new" \Magento\Framework\Mail\Message instance:

    /**
     * Reset object state
     *
     * @return $this
     */
    protected function reset()
    {
        $this->message = $this->objectManager->create(\Magento\Framework\Mail\Message::class);
        $this->templateIdentifier = null;
        $this->templateVars = null;
        $this->templateOptions = null;
        return $this;
    }

once a mail was previously created, the object returned from the ObjectManager won't be a new, uniinitalized object and thus needs at least be reset properly. or get rid of the object manager stuff as everyone seems to preach nowadays.

[andypieters]

Having the same issue in 2.2.4.
Our payment plugin sends the order confirmation and invoice when the payment is completed.
The order confirmation is sent, the invoice is not.
This worked in the past and it clearly is a bug in magento core.
Iam using \Magento\Sales\Model\Order\Email\Sender\OrderSender and \Magento\Sales\Model\Order\Email\Sender\InvoiceSender to sent those emails.
After debugging, i found out that if i don't send the order confirmation, the invoice email works.
Also got the exception: "From Header set twice"

[heldchen]

here's a core patch: bugfix--allow-more-than-one-email-being-sent.zip

please refrain from commenting the ugliness of the patch - mostly due to the stupidity of this bug. I'm sure someone can come up with a nicer solution eventually by getting rid of this dependency injection/object manager mess introduced in 2.2.4.

[andypieters]

@heldchen Thanks.
But the thing is, i myself am not using magento, i just developed a payment plugin for our clients.
I have the feeling that @dimonovp just closed this issue without even looking at it.
@magento-engcom-team Please have a look at this and re-open this issue. We know about the forums and stack exchange, but this clearly is a bug.

[henk-hypershop]

Is a bug indeed. Please re-open this issue.

[gewaechshaus]

Same here...

[vlauria]

With Magento 2.2.4 the issue is in Magento\Framework\Mail\Template\TransportBuilderByStore, after setFrom not reset header from.

Add this function:

protected function reset()
{
        $this->message = $this->objectManager->create(\Magento\Framework\Mail\Message::class);
        return $this;
 }

and replace function setFromByStore with

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

[heldchen]

@vlauria this will still instantiate a new message each time setFromByStore is called, and as such break multiple sending. check the zip I've added a few comments above for a working patch.

[david-kominek]

@magento-engcom-team Could we have this re-reviewed or clarification added? It seems to be an actual issue with a fairly simple solution.