sendgrid / sendgrid-csharp

The Official Twilio SendGrid C#, .NetStandard, .NetCore API Library
https://sendgrid.com
MIT License
1.09k stars 586 forks source link

Async broken in library, causing deadlocks and responses not returning in non-console apps #235

Closed hunglee closed 8 years ago

hunglee commented 8 years ago

Hi, I'm using the example code to send email on MVC web app but it not response on the line dynamic response = sg.client.mail.send.post(requestBody: mail.Get()); even it has been sent successfully. Am I missing something?

String apiKey = "APIKEY";
dynamic sg = new SendGrid.SendGridAPIClient(apiKey);

Email from = new Email("test@example.com");
String subject = "Hello World from the SendGrid CSharp Library";
Email to = new Email("test@example.com");
Content content = new Content("text/plain", "Textual content");
Mail mail = new Mail(from, subject, to, content);

 dynamic response = sg.client.mail.send.post(requestBody: mail.Get());

Note: the example code work fine on console app only (response after post)

Thanks

trinathm commented 8 years ago

I am also facing same issue. Its working fine with console app. but when integrated with MVC application, its not returning response.

thinkingserious commented 8 years ago

@hunglee thanks for letting us know. We will be digging deeper to figure out what is going wrong.

Any additional details are appreciated.

@hunglee @trinathm Even though the response is not returned, does the email get sent?

hunglee commented 8 years ago

@thinkingserious yes, the email was sent successfully. The response is also not returned with Web API and Classic Windows Form. Thanks

trinathm commented 8 years ago

@thinkingserious Yes, Email are being sent successfully, but after posting mail, its not returning response.

ChrisRacki commented 8 years ago

same here...

joantorres commented 8 years ago

@thinkingserious Any estimation on when this bug will be fixed?? I was just integrating this library into a new project and it doesn't work at all, but it's quite urgent and if you don't have a hot fix by tomorrow I will have to go find another alternative. I tried to use the previous version 6.3.4 but it does not support templates, so it doesn't work for me. Thanks for the help!

thinkingserious commented 8 years ago

Hello Everyone,

@hunglee @trinathm @ChrisRacki @joantorres @jahmai @walmon

First, thanks for your feedback and patience.

I've done some digging and I can't reproduce in the console app (as mentioned, it seems to work there) so here is what I'm thinking (I would love to get some feedback):

The API call happens here which calls this function.

So perhaps this call dynamic response = sg.client.mail.send.post(requestBody: mail.Get()); should be dynamic response = await sg.client.mail.send.post(requestBody: mail.Get());

Thoughts?

I still have more digging to do, as I don't have an environment where I see this error reproduced (if anyone can help with that, I would be greatly appreciative).

matthewbcline commented 8 years ago

I can reliably reproduce this in a Web API project. Here is my method:

[Route("sendEmail"), HttpPost, AllowAnonymous]
public virtual async Task<IHttpActionResult> SendEmail()
{
    var apiKey = "my api key";
    var from = new Email("from@example.com");
    var subject = "Subject";
    var to = new Email("to@example.com");
    var textContent = new Content("text/plain", "Content");
    var mail = new Mail(from, subject, to, textContent);

    dynamic sendGridClient = new SendGridAPIClient(apiKey);
    // All three of these calls never return. The email message is sent, however.
    dynamic response = sendGridClient.client.mail.send.post(requestBody: mail.Get());
    dynamic response = await sendGridClient.client.mail.send.post(requestBody: mail.Get());
    dynamic response = await (sendGridClient.client.mail.send.post(requestBody: mail.Get())).ConfigureAwait(false);

    // This line is never executed.
    return this.Ok();
}

I cannot use the client library at all until this is resolved. I'll be making "raw" HTTP requests instead using HttpClient.

thinkingserious commented 8 years ago

Thanks @matthewbcline!

What does the code look like that calls SendEmail()?

Seems like we need to have .ConfigureAwait(false); here and here.

matthewbcline commented 8 years ago

@thinkingserious SendEmail() is a Web API method; it's triggered by an HTTP POST. So it's called by the Web API routing framework.

I had a sneaking suspicion that the solution would involve ConfigureAwait(false). :wink:

thinkingserious commented 8 years ago

Agreed, I will try that fix and report back. Thanks!

alanhouck commented 8 years ago

@thinkingserious I was experimenting with the ConfigureAwait(false) earlier today and had no joy. Thankfully I checked this thread and tried adding it to the exact spots in the code you mentioned and it worked for me. Thanks!

thinkingserious commented 8 years ago

@alanhouck thanks for confirming!!!

I will push out an update and I'd like to swag out everyone on this thread. Please send your T-shirt size and mailing address to dx@sendgrid.com :)

thinkingserious commented 8 years ago

This issue is fixed in version 7.0.2: https://github.com/sendgrid/sendgrid-csharp/releases/tag/v7.0.2

joantorres commented 8 years ago

@thinkingserious This worked! Thanks a lot for the quick fix, and for the T-shirt ;-)

PranKe01 commented 8 years ago

Now, which one is the correct call?

dynamic response = sendGridClient.client.mail.send.post(requestBody: mail.Get()); dynamic response = await sendGridClient.client.mail.send.post(requestBody: mail.Get()); dynamic response = await (sendGridClient.client.mail.send.post(requestBody: mail.Get())).ConfigureAwait(false);

thinkingserious commented 8 years ago

@PranKe01 whether or not you include the await keyword depends on your desired behavior, please see: https://msdn.microsoft.com/en-us/library/hh156528.aspx

PranKe01 commented 8 years ago

When using dynamic response = await sendGridApi.client.mail.send.post(requestBody: mail.Get()); instead of dynamic response = sendGridApi.client.mail.send.post(requestBody: mail.Get()); I get an exception.

thinkingserious commented 8 years ago

@PranKe01 what is the exception you are receiving?

PranKe01 commented 8 years ago

An exception of type 'Microsoft.CSharp.RuntimeBinder.RuntimeBinderException' occurred in System.Web.Mvc.dll but was not handled in user code

Additional information: SendGrid.CSharp.HTTP.Client.Response enthält keine Definition für GetAwaiter.

thinkingserious commented 8 years ago

@PranKe01 do you mind creating a new ticket for that specific issue? If I don't hear back I'll go ahead and create it.

thinkingserious commented 8 years ago

@PranKe01 check out this solution: https://github.com/sendgrid/sendgrid-csharp/issues/259#issuecomment-229396688

mhagesfeld commented 7 years ago

@thinkingserious - Not sure if this is related, but I have an issue where I built a library to send emails. Testing it, it works fine. If I take the code into my MVC project, it works fine. However, if I include the package in my MVC project and make the call to the library, I do not get a response. Any idea what might be happening? Should I open a separate issue?

Library code:

public async Task<string> SendEmail(string toAddresses, string fromEmailAddress, string fromName, string bccAddresses, string subject, string templateId, string bodyContent, Dictionary<string,string> substitutions)
{
 var message = new SendGridMessage();
            message.From = new EmailAddress(fromEmailAddress, name: string.IsNullOrEmpty(fromName) ? null : fromName);
            message.Subject = subject;
            if(!string.IsNullOrEmpty(templateId)) message.TemplateId = templateId;

            message.AddTos(GetEmailAddressListFromCommaSeparatedEmails(toAddresses));
            if(!string.IsNullOrEmpty(bccAddresses)) message.AddBccs(GetEmailAddressListFromCommaSeparatedEmails(bccAddresses));

            message.AddSubstitutions(substitutions);

            var client = new SendGridClient(apiKey);
            var response = await client.SendEmailAsync(message);
            return response.StatusCode.ToString();
}

MVC code:

        public async Task<string> SendValidationEmail(string toAddress, string code, Guid invitationId)
{
var verificationUrl = string.Format("{0}/account/verify/{1}", ConfigurationManager.AppSettings["SiteUrl"], invitationId.ToString());
            var substitutions = new Dictionary<string, string>
            {
                { "-verificationurl-", verificationUrl},
            {"-code-", code }
        };
            var result = sender.SendEmail(
                toAddresses: toAddress,
                fromEmailAddress: ConfigurationManager.AppSettings["FromEmail"],
                fromName: ConfigurationManager.AppSettings["FromName"],
                subject: "Your Calfee Connect Verification Code",
              templateId: "94bae6c6-2d25-47b2-9198-21215e76d4a9",
              bccAddresses: string.Empty,
              bodyContent: string.Empty,
              substitutions: substitutions
              ).Result;
return result;
}
thinkingserious commented 7 years ago

Hello @mhagesfeld,

Please do open a separate issue and we will investigate. Thanks!

Korijn commented 7 years ago

This is still an issue!

thinkingserious commented 7 years ago

Hi @Korijn,

Could you please provide further detail so that I may assist?

  1. What version of the SDK are you using?
  2. Sample code that is causing trouble?

Thanks!

With Best Regards,

Elmer

tawani commented 7 years ago

@thinkingserious This is still an issue. SendEmailAsync is not responding (ASP.NET Web API C#). My app just hangs and no response is ever returned

It, however, works on my test Console app

Sendgrid v9.2.0 .NET Framework 4.6.1

var client = new SendGridClient(ApiKey);
var msg = new SendGridMessage()
{
    From = new EmailAddress("no-reply@mysite.com", "MySite Team"),
    Subject = email.Subject,
    PlainTextContent = StripHtml(email.CreateMailMessage().Body),
    HtmlContent = email.CreateMailMessage().Body
};
msg.AddTo(new EmailAddress(email.To));

msg.SetFooterSetting(true,"MySite &copy; 2016", "<strong>MySite &copy; 2016</strong>");
msg.SetClickTracking(true, true);

var response = await client.SendEmailAsync(msg);
//response never gets returned
Korijn commented 7 years ago

The call to ConfigureAwait(false) that is made internally is messing up my program. SendGrid devs, why is that call there? If you could make the boolean a parameter so users can determine the value passed to ConfigureAwait, that would resolve the problem for me.

thinkingserious commented 7 years ago

Hi @tawani,

I will address this issue more fully shortly, but for now, please try this:

var response = await client.SendEmailAsync(msg).ConfigureAwait(false);

@Korijn,

I like the idea of making that configurable. As to why we did it, we had a long conversation about this, with reference to this article that ultimately resulted in this solution. I'll come back with further details later. I just jumped in real quick to hopefully solve @tawani's issue.

Thanks!

thinkingserious commented 7 years ago

@tawani,

Were you able to resolve the issue with my suggestion? As part of this issue we will be updating our examples to help avoid this type of issue in the future. Thanks again for your feedback!

@Korijn,

Here is the issue to implement a configurable ConfigureAwait. Thanks for the suggestion! Could you provide some reasoning on why we would want ConfigureAwait to be true? I'd like to add that to the issue. Thanks!

tawani commented 7 years ago

@thinkingserious var response = await client.SendEmailAsync(msg).ConfigureAwait(false); Did not work either. No emails were sent and none of the emails even show up on my SendGrid dashboard

thinkingserious commented 7 years ago

Hello @tawani,

Can you share how you are calling SendValidationEmail?

Also, perhaps this example might help.

tawani commented 7 years ago

Not sure about SendValidationEmail

This is how I am calling the service:

var msg = MailHelper.CreateSingleEmail(from, to, email.Subject, plainTextContent, htmlContent); var response = await client.SendEmailAsync(msg).ConfigureAwait(false);

Please advice!

On Sun, May 14, 2017 at 1:07 PM, Elmer Thomas notifications@github.com wrote:

Hello @tawani https://github.com/tawani,

Can you share how you are calling SendValidationEmail?

Also, perhaps this example https://github.com/paritoshmmmec/Sendgrid-WebSample might help.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sendgrid/sendgrid-csharp/issues/235#issuecomment-301325828, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOWpd4Nwgzuwq6XuCf_V2WQLeD5QOgqks5r5zS7gaJpZM4I2Cd2 .

thinkingserious commented 7 years ago

Hello @tawani,

I have tried to reproduce, but I am not getting any errors, the button gets updated with the string "Accepted" and the email gets delivered with the following code:

using System;
using SendGrid;
using SendGrid.Helpers.Mail;
using System.Threading.Tasks;

namespace TestingWebForms
{

    public partial class Default : System.Web.UI.Page
    {
        public void button1Clicked(object sender, EventArgs args)
        {
            var response = Execute().Result;
            Response result = response;
                        button1.Text = result.StatusCode.ToString();
        }

        public static async Task<Response> Execute()
        {
            var apiKey = Environment.GetEnvironmentVariable("SENDGRID_API_KEY");
            var client = new SendGridClient(apiKey);
            var from = new EmailAddress("dx@sendgrid.com", "DX ");
            var subject = "Hello World from the SendGrid CSharp SDK!";
            var to = new EmailAddress("example@example.com", "Elmer Thomas");
            var plainTextContent = "Hello, Email!";
            var htmlContent = "<strong>Hello, Email!</strong>";
            var msg = MailHelper.CreateSingleEmail(from, to, subject, plainTextContent, htmlContent);
                        var response = await client.SendEmailAsync(msg);
            return response;
        }
    }
}

Please take a look and let me know what you think.

tawani commented 7 years ago

the button gets updated with the string "Accepted" Yeas. But the email NEVER gets delivered. That is the problem. My emails never get delivered. They don't even show up in my Sendgrid dashboard.

Please advice!

On Mon, May 15, 2017 at 4:03 PM, Elmer Thomas notifications@github.com wrote:

Hello @tawani https://github.com/tawani,

I have tried to reproduce, but I am not getting any errors, the button gets updated with the string "Accepted" and the email gets delivered with the following code:

using System; using SendGrid; using SendGrid.Helpers.Mail; using System.Threading.Tasks;

namespace TestingWebForms {

public partial class Default : System.Web.UI.Page { public void button1Clicked(object sender, EventArgs args) { var response = Execute().Result; Response result = response; button1.Text = result.StatusCode.ToString(); }

  public static async Task<Response> Execute()
  {
      var apiKey = Environment.GetEnvironmentVariable("SENDGRID_API_KEY");
      var client = new SendGridClient(apiKey);
      var from = new EmailAddress("dx@sendgrid.com", "DX ");
      var subject = "Hello World from the SendGrid CSharp SDK!";
      var to = new EmailAddress("example@example.com", "Elmer Thomas");
      var plainTextContent = "Hello, Email!";
      var htmlContent = "<strong>Hello, Email!</strong>";
      var msg = MailHelper.CreateSingleEmail(from, to, subject, plainTextContent, htmlContent);
                    var response = await client.SendEmailAsync(msg);
      return response;
  }

} }

Please take a look and let me know what you think.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sendgrid/sendgrid-csharp/issues/235#issuecomment-301588409, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOWpePJ2XIZ9vt22OO8j9YRXm-wpNE4ks5r6K92gaJpZM4I2Cd2 .

thinkingserious commented 7 years ago

Hi @tawani,

It sounds like something may be wrong with your account. Could you please reach out to our support team so that they can take a deeper look? Thanks!

tawani commented 7 years ago

I have already opened a tciket and contacted support since Sunday. But they still have resolved anything.

Please advice!

Tawani

On Mon, May 15, 2017 at 4:48 PM, Elmer Thomas notifications@github.com wrote:

Hi @tawani https://github.com/tawani,

It sounds like something may be wrong with your account. Could you please reach out to our support team https://support.sendgrid.com so that they can take a deeper look? Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sendgrid/sendgrid-csharp/issues/235#issuecomment-301600318, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOWpdifIyKAizZQt34OwcANed8nYvtAks5r6LowgaJpZM4I2Cd2 .

thinkingserious commented 7 years ago

@tawani,

Can you please provide your support ticket number? Thanks!

tawani commented 7 years ago

@thinkingserious The ticket is: #1094028

Thx

thinkingserious commented 7 years ago

Hi @tawani,

Did you try the solution offered regarding your lists?

tawani commented 7 years ago

Which solution? None works. I have been manually resetting emails all day because Sendgrid is not working.

thinkingserious commented 7 years ago

The last one sent by our support team regarding certain emails on your lists, I believe it was sent on Saturday.

What do you mean by manually resetting emails? Do you mean resending?

Do you mind sending more of your source code so that we may try to reproduce?

thinkingserious commented 7 years ago

If you prefer not to post it here, please send to dx@sendgrid.com.

tawani commented 7 years ago

I have tried everything you sent. No email is delivered. The only email that gets delivered is the one sent to "tawani@gmail.com" (my own email address). Emails don't get delivered or even displayed on my Sendgrid dashboard.

Please advice!

T

On Wed, May 17, 2017 at 5:20 PM, Elmer Thomas notifications@github.com wrote:

If you prefer not to post it here, please send to dx@sendgrid.com.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sendgrid/sendgrid-csharp/issues/235#issuecomment-302235010, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOWpTpFOoGDr3qJfzZ8HOoDpwy90xrpks5r62SfgaJpZM4I2Cd2 .

thinkingserious commented 7 years ago

Hi @tawani,

Do you mind sending more of your source code so that we may try to reproduce?

If you prefer not to post it here, please send to dx@sendgrid.com.

Thanks!

tawani commented 7 years ago

Here you go:

public async Task SendAsync(Common.Emailing.Email email) { var client = new SendGridClient(ApiKey); var msg = new SendGridMessage() { From = new EmailAddress("no-reply@curistech.com", "CurisTech Team"), Subject = email.Subject, PlainTextContent = StripHtml(email.CreateMailMessage().Body.Replace("
", "\r\n").Replace("
", "\r\n")), HtmlContent = email.CreateMailMessage().Body }; msg.AddTo(new EmailAddress(email.To));

msg.SetFooterSetting(
            true,
            "CurisTech &copy; 2016",
            "<strong>CurisTech &copy; 2016</strong>");
msg.SetClickTracking(true, true);

var response = await client.SendEmailAsync(msg);
//return response;

}

On Thu, May 18, 2017 at 9:31 AM, Elmer Thomas notifications@github.com wrote:

Hi @tawani https://github.com/tawani,

Do you mind sending more of your source code so that we may try to reproduce?

If you prefer not to post it here, please send to dx@sendgrid.com.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sendgrid/sendgrid-csharp/issues/235#issuecomment-302404174, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOWpbB2BCfxDj3HW5GRXH_a8jQaEWVcks5r7EghgaJpZM4I2Cd2 .

thinkingserious commented 7 years ago

Hi @tawani,

How are you calling SendAsync?

tawani commented 7 years ago
    public void Send(Common.Emailing.Email email)
    {
        SendAsync(email).Wait();
    }

On Thu, May 18, 2017 at 12:38 PM, Elmer Thomas notifications@github.com wrote:

Hi @tawani https://github.com/tawani,

How are you calling SendAsync?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sendgrid/sendgrid-csharp/issues/235#issuecomment-302465526, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOWpXLk_WdnThLrCQKNqyUOuFsC5dbvks5r7HP8gaJpZM4I2Cd2 .

thinkingserious commented 7 years ago

Please try:

  1. Uncomment return response
  2. Replace SendAsync(email).Wait(); with
    SendAsync(email).Result;
    Response result = response;
    var statusCode = result.StatusCode.ToString(); // log this value appropriately so that you can read the result
  3. Replace public async Task SendAsync(Common.Emailing.Email email) with public static async Task<Response> SendAsync(Common.Emailing.Email email)

Also, when you say "The only email that gets delivered is the one sent to "tawani@gmail.com". Do you mean that when you try your code with that email it works fine, but when you try again with any other email it does not work? Could you try sending to dx@sendgrid.com?

tawani commented 7 years ago

I used "tawani@gmail.com" and a bunch of other emails on a test Console app. And only "tawani@gmail.com" was delivered. See screenshot below - Only console apps get logged by Sendgrid:

On Thu, May 18, 2017 at 12:50 PM, Elmer Thomas notifications@github.com wrote:

Please try:

  1. Uncomment return response
  2. Replace SendAsync(email).Wait(); with

SendAsync(email).Result;Response result = response;var statusCode = result.StatusCode.ToString(); // log this value appropriately so that you can read the result

  1. Replace public async Task SendAsync(Common.Emailing.Email email) with public static async Task SendAsync(Common.Emailing.Email email)

Also, when you say "The only email that gets delivered is the one sent to " tawani@gmail.com". Do you mean that when you try your code with that email it works fine, but when you try again with any other email it does not work? Could you try sending to dx@sendgrid.com?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sendgrid/sendgrid-csharp/issues/235#issuecomment-302468666, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOWpeIpHIK8eKh8_rXF6d6Z4YTkwvBzks5r7HbEgaJpZM4I2Cd2 .