Laravel Version: 5.5.19
PHP Version: 7.2.0
Description:
When attaching raw data using attachData
to a Mailable and sending the mail through the queue using later()
, an InvalidPayloadException
will be thrown. The error code thrown is 5, indicating malformed UTF-8 characters.
The method seems to be thrown in the createPayload()
method:
protected function createPayload($job, $data = '')
{
$payload = json_encode($this->createPayloadArray($job, $data));
if (JSON_ERROR_NONE !== json_last_error()) {
throw new InvalidPayloadException(
'Unable to JSON encode payload. Error code: '.json_last_error()
);
}
return $payload;
}
Steps To Reproduce:
Mailable
attachData
Mail::later(...)
Why not use Mailable.attach for attaching files?
attach(string $file, array $options = [])
Attach a file to the message.
i believe its expected behavior since file data in memory cant be json encoded properly unless you use some kind of encoding like base64
or read the file and get the hex string for that, but then again it will probably break the mail body and won't render in the mail client.
I could probably refactor my code to to this, but if this would be the advised solution, perhaps this should be validated and crafted into a custom exception?
@ArondeParon no not really its invalid JSON and the exception message is detailed. Otherwise it would have to "detect" somehow that the parameter its an file handle or an file that has been read into a string, which i believe its just way too much unnecessary work for such an function when other methods are clearly made to cover the case of adding attachments.
Also look into .attachData
$this attachData(string $data, string $name, array $options = [])
Attach in-memory data as an attachment.
The point of attachData
is to add an email attachment, which, by definition, is a file. The main difference vs. attach
is that this method attaches it from memory.
If this is incompatible with the Queue, the documentation should reflect this, since the attach
and attachData
methods do not refer to JSON encoding. These are all Queue internals. Although the exception indicates what is causing the issue, there is little reason to send developers off into a path that is a dead-end.
Feel free to open a PR with the suggested changes, or move the discussion to laravel/internals, this doesn't seem like a bug that needs to be fixed.
@themsaid Why is this not a bug? I don't get it.
You might say you won't fix it because you have other priorities, but this does sound like a bug to me in all fairness.
I just ran in to the same issue myself. We have 1TB+ of files in an S3 bucket for a customer. At the moment, we are sending email to their users with download links. The customer has many requests from their users to send the documents in attachment, so we want to do this:
At this time, it's almost impossible to do this...
->addAttachment()
only works with a local path whereas ->attachData()
does seem to be incompatible with the queueing.
I believe this should be fixed in the mailable class itself; in order to avoid having a too large payload on the queued items, I would prefer that ->addAttachment
works with any kind of storage. It should be transparant for the developer.
So to summarise, I don't understand why this would not qualify as a bug... :-)
@ArondeParon You happen to have found a workable and elegant solution to the issue?
@denjaland sadly, no. We worked around the issue by skipping the queue alltogether in this case, but the underlying issue was not solved.
Well - the queueing issue is less to an issue to me than not being able to use addAttachment with a s3 stored file on the mailable class... maybe I should raise an issue for that instead. Let me first check if I can find anything first.
Thanks for gettibg back, @ArondeParon!
Hi all,
i was able to circle arround this issue by giving my Mailable class all that was needed to produce the attatchment and than attatch that to the email (Mailable class) on the build method. My build method looks like this:
/**
* Build the message.
*
* @return $this
*/
public function build()
{
$orderPDF = new OrderPDF();
$this->attachData($orderPDF->render($this->order->id), "order_nr{$this->order->nr}.pdf", ['mime' => 'application/pdf']);return $this->view('email.order', [ 'user' => $this->user, 'order' => $this->order, ]); }
$this->order and $this->user are passed to the constructor.
Hope this helps someone
@ArondeParon I Found a work around for using queue. I am using laravel 7.*
I create a class with a constructor and a __toString method and passed the class to attachData.
class Pdf
{
protected string $data;
public function __construct($data)
{
$this->data = base64_encode($data);
}
public function __toString()
{
return base64_decode($this->data);
}
}
Then used it like so:
$mail->attachData(new Pdf('pdf stream'),'filename.pdf',['mime' => 'application/pdf']);
This works as expected;
@iruku Thank you for the lean solution.
This is an issue waiting to be fixed.
Most helpful comment
The point of
attachData
is to add an email attachment, which, by definition, is a file. The main difference vs.attach
is that this method attaches it from memory.If this is incompatible with the Queue, the documentation should reflect this, since the
attach
andattachData
methods do not refer to JSON encoding. These are all Queue internals. Although the exception indicates what is causing the issue, there is little reason to send developers off into a path that is a dead-end.