I feel like we're changing a lot of code and logic just to fix what I would imagine is a much simpler bug? 👀 What is the minimum possible change to fix the issue?

While fixing the misuse of the DataPart API, I found that the tests also needed adjustments. I thought it would be best to fix everything properly.

I can't recreate this locally. I setup a failover mail configuration where the first connection is SMTP with invalid credentials and the second connection is a different SMTP configuration with valid credentials.

It correctly failed over and my image was present in the email:

Hi Taylor,

Thank you so much for taking the time to test this - I really appreciate you looking into it personally despite your busy schedule.
I sincerely apologize for not providing a simple, standalone reproduction case upfront. I should have made it easier for you to verify the issue.

I've now created a standalone reproduction repository:
https://github.com/yinheli/mail-failover-with-embedded-image-reproduction

Why your SMTP test didn't reproduce the bug:

SMTP fails at connection time (before rendering), so the Mail object never calls
toIterable() and the bug doesn't trigger. HTTP transports like Mailgun/SES render
the message first, which is when the addPart() bug manifests.

The reproduction repo uses invalid Mailgun credentials (not SMTP) as primary to trigger the render before failover.

Alright - was able to recreate this finally and confirm your fix does indeed resolve the issue.

Thanks!

Hi,

it seems that this PR might break inline image embedding in emails. This Blade template code used to work before:

<img src="{{ $message->embed(resource_path('img/logo.png')) }}"
     width="140" alt="Alt text" style="height:auto; max-width:100%;" />

The src is changed to "cid:..." which should then be replaced with the base64 encoded file in Mailer::replaceEmbeddedAttachments. The code compares $attachment->getFilename() against the cid in src attribute, but the file name is different now. Perhaps the comparison in replaceEmbeddedAttachments should now be if ($attachment->getContentId() === $image) { ... }?