bbottema/simple-java-mail

Always invoke async success/exception handlers even if set after sending email (behaving more like promises/futures)

Dornaut opened this issue · 4 comments

I'm using simple-java-mail 6.4.3 with batch-module and try to understand how is it guaranteed for onSuccess and onException handlers that their setting will be performed before their usage?

AsyncOperationHelper contains comment

// by the time the code reaches here, the user would have configured the appropriate handlers

But if I correctly understand it's still possible (if inner NamedRunnable will be executed fast) that will be executed before the user will set corresponding callback in main thread

E.g. I tried to perform simple test based on example from site:

AsyncResponse asyncResponse = mailer.sendMail(email, true);
asyncResponse.onSuccess(() -> System.out.println("Success"));
asyncResponse.onException((e) -> System.err.println("error"));

In this case the callback was executed successfully.
But if the main thread will sleep for some time, e.g.

AsyncResponse asyncResponse = mailer.sendMail(email, true);
Thread.sleep(10000);
asyncResponse.onSuccess(() -> System.out.println("Success"));
asyncResponse.onException((e) -> System.err.println("error"));

The callback was not executed at all because

was called when onSuccess method was not called yet and successHandler was still null.

Do I understand something wrong or is it a bug?

This can probably be solved if callbacks will be passed directly into the sendMail method and then directly into the AsyncResponseImpl constructor

asyncResponseRef.set(new AsyncResponseImpl(executorService.submit(new NamedRunnable(processName) {

Excellent report, thank you! I will look into it this week. Seems like a bug to me.

@bbottema Thank you! We would really appreciate it.

So the assumption is that the user would set the handlers right after calling the send method, which should always take less time than sending the email, even when it's set to skip sending and only log. So in practice this shouldn't be an issue, but I agree it's not a very elegant approach.

I'll change this behavior to be more in line with how promised/futures work so the handlers will always be invoked.

Released in 6.4.4.