1

I have set up Hangfire correctly. I am able to run the following code from postman:

 [HttpPost("appointments/new")]
 public async Task<IActionResult> SendMailMinutely()
 {
     RecurringJob.AddOrUpdate(() => Console.WriteLine("Recurring!") Cron.Minutely);
     await Task.CompletedTask;
     return Ok();
 }

When I hit this API point, this works fine. What I would like to do is to run my email controller using the same code above. My modified SchedulersController code is:

[Route("/api/[controller]")]
public class SchedulersController : Controller
{
    private readonly MailsController mail;
    public SchedulersController(MailsController mail)
    {
        this.mail = mail;
    }

    [HttpPost("appointments/new")]
    public async Task<IActionResult> SendMailMinutely()
    {
        RecurringJob.AddOrUpdate(() => mail.SendMail(), Cron.Minutely);
        await Task.CompletedTask;
        return Ok();
    }
}

And My MailsController is:

[HttpPost("appointments/new")]
public async Task<IActionResult> SendMail()
 {
    var message = new MimeMessage ();
    message.From.Add (new MailboxAddress ("Test", "test@test.com"));
    message.To.Add (new MailboxAddress ("Testing", "test@test123.com"));
    message.Subject = "How you doin'?";

    message.Body = new TextPart ("plain") {
        Text = @"Hey Chandler,
                I just wanted to let you know that Monica and I were going to go play some paintball, you in?
                -- Joey"
    };

     using (var client = new SmtpClient ()) {
     client.ServerCertificateValidationCallback = (s,c,h,e) => true;

    client.Connect ("smtp.test.edu", 25, false);

    await client.SendAsync (message);
            client.Disconnect (true);
        }


        return Ok();
    }

The error message I receive is:

An unhandled exception has occurred while executing the request. System.InvalidOperationException: Unable to resolve service for type 'Restore.API.Controllers.MailsController' while attempting to activate 'Restore.API.Controllers.SchedulersController'

How can I use my MailsController so that I can schedule emails to send using Hangfire? Any help will be greatly appreciated.

Fabian
  • 279
  • 2
  • 4
  • 16
  • Why did you mark your methods as async if you're just going to await a completed task? Why not just return an `IActionResult` instead of `Task`? – mason Oct 23 '18 at 17:29
  • Because it has to be `async` since I have multiple requests. Some requests take some time. This mail request shouldn't be waiting other to be completed. – Fabian Oct 23 '18 at 17:32
  • 2
    Why don't you encapsulate your mail sending logic into a separate service? Why are you putting that directly in a controller's action method? It doesn't belong there. – mason Oct 23 '18 at 17:32
  • The only async code I see you calling in SchedulersController is `await Task.CompletedTask;`. – mason Oct 23 '18 at 17:33
  • I would recommend moving the logic in SendMail to a separate class, an EmailService that would be injected into both controllers. Injecting a controller into another is something of an anti-pattern. In the meantime, see the following answer for how to solve your issue. https://stackoverflow.com/a/49372654/10296119 – CPerson Oct 23 '18 at 17:35
  • The most confusing part about this is why a controller is responsible for orchestrating a scheduled job/service – Mark C. Oct 23 '18 at 17:35

2 Answers2

3

The proper way to do this is to move your mail sending logic into a separate service.

// We need an interface so we can test your code from unit tests without actually emailing people
public interface IEmailService
{
    async Task SendMail();
}

public EmailService : IEmailService
{
    public async Task SendMail()
    {
        // Perform the email sending here
    }
}

[Route("/api/[controller]")]
public class SchedulersController : Controller
{
    [HttpPost("appointments/new")]
    public IActionResult SendMailMinutely()
    {
        RecurringJob.AddOrUpdate<IEmailService>(service => service.SendMail(), Cron.Minutely);
        return Ok();
    }
}

You'll need to make sure you've configured Hangfire for IoC as described in their documentation, so that it can resolve an IEmailService.

mason
  • 31,774
  • 10
  • 77
  • 121
  • You're right. The way I described above is simply against the pattern. It works now as expected. Thanks for the help! – Fabian Oct 23 '18 at 17:54
  • I have a question here that might be to general to give one answer: lets say that he would not create the Hangfire job, but just used the SenMail method without awaiting it... wouldn't that be faster than creating job? – Eru May 11 '22 at 19:51
  • @Eru Of course it'd be faster. However, what if sending that email fails? What if the mail server is temporarily down, or there's some other networking blip between your app and the mail server that causes the SendMail to fail. If you don't await the task, then how will you be notified of the exception? You won't. Processing it in a background job makes sense, because you need it to be durable (it's simple to retry a Hangfire job if it fails) and you also don't necessarily want to wait for the whole SMTP handshake to complete, as it may unnecessarily slow down request processing. – mason May 11 '22 at 20:06
  • @Eru Calling an async method without awaiting is almost never a good idea, since you won't have a chance to catch any thrown exceptions. The exceptions just get lost. While one can maybe make the argument that Hangfire is overkill for sending email in simple cases, you don't really have a leg to stand on if you want to argue that simply not awaiting async method calls is the proper thing to do. – mason May 11 '22 at 20:08
  • @Eru See [this Fiddle](https://dotnetfiddle.net/7SnI85) demonstrating how when you don't await async method calls, you lose the exception. – mason May 11 '22 at 20:15
  • Generally I know that not using await is a bad idea: I agree with that and with you. But...: when using hangfire for sending email and sending fails, then what? According to documentation I can setup hangfire to retry the job when it fails. Where I am getting to is that both: "without await" and "send email via hangfire" are NOT solving the failure of sending email :) – Eru May 11 '22 at 22:04
  • @Eru Of course not. What they're doing though is making it so that 1) you know a failure happened and 2) you have an easy way of retrying failed background tasks (sending emails). Obviously if your network is down or your email server is crashed, you'll need to solve those issues. – mason May 11 '22 at 23:17
0

This is related to Dependency Injection in the Core Framework. You need to ensure that you register your dependencies in your startup.cs under ConfigureService method.

Not sure if this is good practice though.

For Controller you can use: services.AddMvc().AddControllersAsServices();

Janus Pienaar
  • 1,083
  • 7
  • 14
  • Well `MailsController` is a controller, not interface so that I can register in the startup class. It uses services which they're are already registered. – Fabian Oct 23 '18 at 17:29