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

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

Closed
hunglee opened this issue Jun 15, 2016 · 66 comments
Labels
status: help wanted requesting help from the community type: bug bug in the library

Comments

@hunglee
Copy link

hunglee commented Jun 15, 2016

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("[email protected]");
String subject = "Hello World from the SendGrid CSharp Library";
Email to = new Email("[email protected]");
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
Copy link

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

@thinkingserious thinkingserious added type: bug bug in the library status: help wanted requesting help from the community labels Jun 15, 2016
@thinkingserious
Copy link
Contributor

@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
Copy link
Author

hunglee commented Jun 16, 2016

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

@trinathm
Copy link

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

@ChrisRacki
Copy link

same here...

@thinkingserious thinkingserious changed the title Example send email code not response after post on web platform Async broken in library, causing deadlocks and responses not returning Jun 16, 2016
@thinkingserious thinkingserious changed the title Async broken in library, causing deadlocks and responses not returning Async broken in library, causing deadlocks and responses not returning in non-console apps Jun 16, 2016
@joantorres
Copy link

@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
Copy link
Contributor

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
Copy link

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("[email protected]");
    var subject = "Subject";
    var to = new Email("[email protected]");
    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
Copy link
Contributor

Thanks @matthewbcline!

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

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

@matthewbcline
Copy link

@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). 😉

@thinkingserious
Copy link
Contributor

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

@alanhouck
Copy link

@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
Copy link
Contributor

@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 [email protected] :)

@thinkingserious
Copy link
Contributor

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

@joantorres
Copy link

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

@PranKe01
Copy link

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
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

@PranKe01 what is the exception you are receiving?

@PranKe01
Copy link

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
Copy link
Contributor

@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.

@tawani
Copy link

tawani commented May 16, 2017

@thinkingserious
The ticket is: #1094028

Thx

@thinkingserious
Copy link
Contributor

Hi @tawani,

Did you try the solution offered regarding your lists?

@tawani
Copy link

tawani commented May 17, 2017

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

@thinkingserious
Copy link
Contributor

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
Copy link
Contributor

If you prefer not to post it here, please send to [email protected].

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

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 [email protected].

Thanks!

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

Hi @tawani,

How are you calling SendAsync?

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

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<Response> SendAsync(Common.Emailing.Email email)

Also, when you say "The only email that gets delivered is the one sent to "[email protected]". 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 [email protected]?

@tawani
Copy link

tawani commented May 18, 2017 via email

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

Hi @tawani,

Did you try making the 3 changes I suggested?

Can you please walk me through step by step what happens when you have the result of "The process just sits there waiting forever"?

For example:

  1. Using Visual Studio version X.X I create a XXXXX project
  2. I use the following code:
    [Sample Code]
  3. When I run the code and set a breakpoint at some point after the await client.SendEmailAsync(msg);, the breakpoint is never reached.

@tawani
Copy link

tawani commented May 18, 2017 via email

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

Did you try making the 3 changes I suggested?

@thinkingserious
Copy link
Contributor

Just before you call var response = await client.SendEmailAsync(msg);, can you tell me the output of msg.Serialize()?

@tawani
Copy link

tawani commented May 18, 2017 via email

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

Your screen shot is not showing up here on GitHub. Perhaps you can upload it somewhere and provide the link.

Take a look at your personalization block:

  "personalizations": [
    {
      "to": [
        {
        }
      ]
    }
  ],

Your email is not getting added.

Can you take a look at msg.AddTo(new EmailAddress(email.To));, specifically, please verify the value of email.To. And for a quick test, you can manually add an email address there.

Thanks!

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

Sounds good, please let us know how it goes. Thanks!

@tawani
Copy link

tawani commented May 18, 2017 via email

@tawani
Copy link

tawani commented May 18, 2017 via email

@thinkingserious
Copy link
Contributor

@tawani,

I'm still not seeing any attachment. GitHub must scrub them from emails.

@tawani
Copy link

tawani commented May 23, 2017 via email

@thinkingserious
Copy link
Contributor

Hello @tawani,

Unfortunately, I still do not see any attachment. Do you mind uploading to a service like Dropbox? So far, I'm unable to reproduce the issue.

Also, we have a new example that might help you.

With Best Regards,

Elmer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: bug bug in the library
Projects
None yet
Development

No branches or pull requests