IOException on mail/send request with duplicate recipients

[TBuc]

Issue Summary

When trying to send an email which has duplicated recipients among all the recipients fields (To, Cc, Bcc), an IOException is thrown.
This is a formally correct since duplicates are not allowed, but it's an occurrence that might both be automatically prevented or provide some eager notification to the user.

Steps to Reproduce

1. Create a new com.sendgrid.Mail object

Mail mail = new Mail(new Email(pFrom), pSubject, mainRecipient, pContent);

2. Add at least two recipients that have the same email address

String mailAddr = "myemail@mydomain.ext";
mail.personalization.get(0).addTo(new Email(mailAddr));
mail.personalization.get(0).addCc(new Email(mailAddr));

3. Create a new com.sendgrid.Request object and set it to create a new mail using the mail at .1

Request request = new Request();
request.method = Method.POST;
request.endpoint = "mail/send";
request.body = mail.build();
com.sendgrid.Response response = sg.api(request);

4. Create a new com.sendgrid.SendGrid service and use it to submit the request created at .3

SendGrid sg = new SendGrid(SENDGRID_APIKEY);
com.sendgrid.Response response = sg.api(request);

5. You'll get an IOException notifying you that the email could not be sent since it contains some duplicate recipients

Technical details:

• sendgrid-java Version: master (latest commit: [b68c90a])
• Java Version: 1.7

Preconditions

If some case exists when a com.sendgrid.Mail object - is it submitted or fetched - can have multiple recipients, then this issue should be withdrawn.

Proposed solution (RFC)

Otherwise, I can think of the following possible solutions:
a. Eagerly throwing an exception when inserting a duplicate com.sendgrid.Email to tos, ccs or bccs.
b. Silently skipping the insertion when a duplicated value is requested to be inserted.

The option .b can then implement one of the following two logics for handling duplicates:
b1. Always skip insertion
b2. If the value is already present in the same collection it's being inserted to, then skip.
Otherwise, keep (or insert) the value in highest priority collection (tos > ccs > bccs) and remove it from the lower priority one if needed.

This would need the com.sendgrid.Email object to implement equals() and hashCode() methods so that they only depend on the Email.mail field (this respects the contract on Overriding Equals and Hashcode - ref Effective Java item #9).

1. The time performance for checking if a List contains an item would be linear, and three lists should be checked for each new addition.

2. Better time performance can be achieved by duplicating the current data structures for tos, ccs, bccs by creating a new data structure recipients that implement the Set interface (com.sendgrid.Email should then implement Comparable over the same fields as its equals method - ref. JavaDocs1.7). This would lead to constant time but would slightly increase space utilization due to introducing a new data structure.

3. The best performance solution would be to convert the tos, ccs, bccs objects - which currently implement a List - to Set objects, but this would be a breaking change and its impact shall be analyzed (I think the performance gain does not worth the candle for this option).

[thinkingserious]

Thanks for the well thought out issue @TBuc!

I think the root issue is that we don't pass up the underlying exception properly, as described by this issue.

Also, it is possible to send an email in sandbox mode, so that you check whether your payload is valid without using an email credit.

Thoughts?

With Best Regards,

Elmer

[TBuc]

Hi @thinkingserious , thank you for your prompt reply.

I think that sandboxing and then recovering still needs a lot of boilerplate coding on client side, no matter whether an Exception or an errorCode are returned.
Such situation might be eagerly handled once and for all in SG library as suggested above - this would avoid clients writing duplicated and possibly flawed code which must rely on an object different than Email (i.e., String) since Email is not comparable.

Recovering from the a lazy exception/errorCode needs to iterate over all the recipients and remove the duplicates. In this case, in place of recovering later, I would still prefer to do what I'm currently doing - maintaining a Set of all the email addresses I've added up to now, and check on my side if an email is already present in such list before adding it to the Mail object, so to avoid duplicates at the beginning. But this is not nice to be done on client side.

If the answer to the precondition question is that duplicated addresses are never possible, then I would suggest to implement this feature as exposed in my initial message. This would apply the principle of eagerly notifying of errors, which helps a lot both with debugging and with handling them.
Otherwise, such feature might be implemented but disabled by default - an additional constructor for Mail object might contain a flag for enabling it.

It would be nice to have some feedbacks on this from other users.

Edit: rewrote since it was a little messy.

[thinkingserious]

Hi @TBuc,

I can confirm that duplicated addresses are never possible.

With Best Regards,

Elmer

[thinkingserious]

This should be addressed here.