Emails not sent, no error

[cpntgt]

Hi,

I have version 1.1.0 (April 12th), although I see version 1.0.2.22938 in the properties. I have copied the example and added some changes for my requirements and emails are not being sent out. I also do not get an error. Anyone has encountered this problem or have an idea?

public void SendEmail(string toEmail, string toName, string subject, string body, string fromName = null, string replyTo = null, Guid trackingId = default(Guid)) { try {

            if (!System.Diagnostics.Debugger.IsAttached)
            {
                var transmission = new Transmission();
                transmission.Content.From.Email = replyTo ?? "test@test.com";
                transmission.Content.From.Name = fromName ?? "test";
                transmission.Content.Subject = subject;
                transmission.Content.Html = body;
                transmission.Options.OpenTracking = true;

                var recipient = new Recipient
                {
                    Address = new Address { Email = toEmail , Name = toName }
                };
                transmission.Recipients.Add(recipient);

                if (!string.IsNullOrEmpty(replyTo))
                    transmission.Content.ReplyTo = replyTo;

                if (trackingId != Guid.Empty)
                    transmission.Metadata = new Dictionary<string, object> { { "TrackingId", trackingId.ToString() }, { "UserName", toName } };

                //client.Transmissions.Send(transmission).Wait();

                Task result = SendTransmission(transmission);
            }

        }
        catch (Exception ex)
        {
            LogHandler.Error(ex);
        }
    }

    private async Task SendTransmission(Transmission transmission)
    {
        client = new Client(key);
        await client.Transmissions.Send(transmission);            
    }

[darrencauthon]

@cpntgt Have you tried the help from here in the readme? This looks like it might be related: https://github.com/SparkPost/csharp-sparkpost#special-note-about-async

I don't see anything in the code that forces a wait for the result.

[LEloffel]

Same problem, with wait(), code never returns.

[darrencauthon]

@LEloffel @cpntgt I have this code working in production -- it does send, and it sends as expected.

As another demo, something I do before every release: I check it against Linqpad as well, just to try it out. It's functioning as expected

I'm not saying that this library can't be improved... but this is a case where I'd have to see the environment to diagnose and debug. I don't think the solution is changing the code right now.

[darrencauthon]

I've thought about offering non-async methods, but... this is a feature of .Net 4.5, and it opens up much better performance and behavior for other consumers of the library. I don't like a bool splitting the library in half.

[cpntgt]

If we forget the async function call, I also tried by simply putting the .Wait(), like stated on the homepage. In the sample code the line is commented out, but if you change it back can you send an email?

client.Transmissions.Send(transmission).Wait(); // Task result = SendTransmission(transmission);

This should work, no?

[darrencauthon]

I think it should work... and it's worked for me.

In the cases where it hasn't, it's always been something external from the library. There's nothing to change in the library itself. It's just using HttpClient to return the asynchronous response.

[darrencauthon]

I just released version 1.1.1. I don't think it would change anything regarding this ticket, but it would be a good chance to reopen.

I'm going to close this ticket just because I cannot reproduce any issues at all. The library works as expected, through multiple avenues (web, linqpad, windows service). I think this issue (and many others) are all tied to misunderstandings regarding Async.

I'd be happy to take another look if we can get a failing test or a concrete example (perhaps a full VS project?) that can be used to reproduce the problem.

[cpntgt]

This is still not working. I updated to the latest version for today (1.1.0) and when I add the .Wait() my Website just waits forever, no exception occurs. I really doubt that I am the only one with this problem.

You said that you tested on a MVC Website. I have the target Framework 4.5 and the System.Net.Http is version (v4.0.30319), is that what you have? What other references are critical for sending the email?

Why don't you have the async and wait handled in the library? I think that would simplify the implementation.

For reference this is what I tried: 1 - client.Transmissions.Send(transmission) - Nothing happends 2 - client.Transmissions.Send(transmission).Wait() - Waits forever 3 - Task task = SendTransmission(transmission); task.Wait();

    private async Task SendTransmission(Transmission transmission)
    {
        client = new Client(key);
        SendTransmissionResponse str = await client.Transmissions.Send(transmission);
    }

Waits forever

[darrencauthon]

@cpntgt

This is not working

It's not the library, it has to be in the way you are using it.

You said that you tested on a MVC Website.

I said so because I have.

I don't fire my emails in an Action so I'm not familiar... but it takes no time to create a web application, so let's see what happens.

The email sent, but the web response hung. Probably due to that Wait() call. So since this action uses an async method, let's do it like everybody uses async methods in C# today...

The email sends, and the response is immediate.

The library works: The problem is in the way you are calling it.

[darrencauthon]

Why don't you have the async and wait handled in the library? I think that would simplify the implementation.

Because .Net 4.5 has a method for making these web calls async. It improves performance and load considerably. If this library didn't use it, it would have a negative effect on all users... and why? Just to make it easier for C# devs who don't know about async/await?

Plus, I'm using the 4.5 HttpClient which does not have a sync method (as far as I know). I could find a way to make it go synchronous, but then I'd be getting Github tickets about people wanting async feature. Damned if I do, damned if I don't. Better to lean towards the better tech.

[cpntgt]

I found the issue. SparkPost does not accept that you send an email from an address that is not from your sending domain. I was using the same address as the reply-to address which most providers will let you. Documentation is found here:

[cpntgt]

I was not sending the emails directly from an action. What I meant is that this was used in a Website context and not a desktop application. In the end, we are simply calling an API but the context is not the same.

Thanks Darren!

[cpntgt]

Because .Net 4.5 has a method for making these web calls async. It improves performance and load considerably. If this library didn't use it, it would have a negative effect on all users... and why? Just to make it easier for C# devs who don't know about async/await?

Right, but users of the library must put the async/await in place in their implementation to make it work. This is not convenient. You should make the SendTransmisson method synchronous and add a SendTransmissionAsync or change the method to Send(Transmission transmission, bool async = false).

[darrencauthon]

@cpntgt

Right, but users of the library must put the async/await in place in their implementation to make it work. This is not convenient.

I actually think it is very convenient; it's the web that's not convenient. We want to make our request and get an immediate response -- right here, right now, right in this current thread, damn the rest of the world. Give me what I want right now. But darn it, reality doesn't adjust to our desires. The reality of this situation is that we're making our request in a process, and our request is over HTTP which has a delay, and we don't know what type of response we're going to get and when it's going to come back.

It's convenient to be lazy and tell the thread to go to sleep until it gets a response. But that can cause all sorts of problems we may not be aware of. The await/async behavior in C# replaces this thread nap with a seam that the application can use to behave much more efficiently.

[darrencauthon]

@cpntgt

You should make the SendTransmisson method synchronous and add a SendTransmissionAsync or change the method to Send(Transmission transmission, bool async = false).

The last thing I'd ever want to do to a library is to split its public API down the middle with a boolean. Any method that takes a boolean as its last argument is doomed to fail, and an API that has two copies of each method is in bad shape.

If you want this behavior, though, I can see one good way to do it in this library. Replace RequestSender, which returns an asynchronous Task<Response>, with a synchronous SynchronousRequestSender which makes the same requests synchronously. This synchronous one will still return a Task<Response>, but that doesn't mean the result has to be asynchronous. Slap the wait inside SynchronousRequestSender, and then return the result with Task.FromResult(response)

With this approach, we could change the library to swap the async one for the sync one based on preference. Then we'd be changing the behavior of the library without changing its public API.

[darrencauthon]

I have to note, though... if a synchronous one is built with a Wait() call, I think we'll start to have issues like the one with the MVC action above.

It seems to me that if I call Wait() inside the action, it should wait until the request is done and then continue. But for some reason, the MVC action just hangs on the result. Calling Wait() inside the library instead of the action is still going to have the same weird result.

[darrencauthon]

@cpntgt @LEloffel Version 1.3.0 has just been released with sync support.

Details here: https://github.com/SparkPost/csharp-sparkpost#special-note-about-async

[ajbeaven]

Sorry, just to clarify - are you saying that it's not possible to Wait() when using the asynchronous command to send emails?

Are you sure this isn't a problem with the library? I've never experienced any problems using Wait() with any other asynchronous library, and this approach is pretty common with legacy systems where it's not feasible or worth it to 'async all the way down'.