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

If file for Embed() is missing, the error is returned but email is still sent #65

Open
wkhere opened this issue Oct 24, 2016 · 6 comments

Comments

@wkhere
Copy link

wkhere commented Oct 24, 2016

Hi!

This library is very helpful.
However, I encountered an inconsistent behaviour which I believe is a bug.

When a file given as an argument to Message.Embed(...) is missing on the filesystem,
there's no error returned (this method returns nothing).
Later, when calling Send(sender, message),
error is returned, but the email is sent (without embedded data of course).

This is confusing - I'd rather expect that when there's an error, an email would not be sent. When it works this way, I cannot really differentiate between such error and other error stopping message from being sent.

@leventogut
Copy link

Same problem exists on attachments as well

@wkhere
Copy link
Author

wkhere commented Nov 17, 2016

yep @leventogut I was suspecting it may affect regular attachments as well.
Any idea of a fix guys? haven't dived into the source, yet

@leventogut
Copy link

haven't checked the code either , I am handling this on upper level not to cause any problems.

have been quite busy so couldn't spend time on it .

@wkhere
Copy link
Author

wkhere commented Jul 21, 2017

Any thoughs on that @alexcesaro ?

In theory there's this CopyFunc registered for both attachments and embeds and this function should return an error which then should be handled properly before sending an email. In addFiles/writeBody methods I can see that some field of messageWriter struct is updated with an error. What next? How it is used then?

@Gamma169
Copy link

I implemented a fix for this issue. Check my pull request. #97

@wkhere
Copy link
Author

wkhere commented Sep 4, 2017

Looks cool. Will integrate it soon with my client code.

@alexcesaro ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants