bbottema/simple-java-mail

Bug: properly Fail-Fast in case of Transport claim timeout in the batch-module, rather than running into NPE further down the line

lopardo opened this issue · 4 comments

When configuring a Mailer with a connection claim timeout like this:

MailerBuilder.withSMTPServer(host, port, username, password)
    .withConnectionPoolMaxSize(1) // for easier testing
    .withConnectionPoolClaimTimeoutMillis(1000)

And a connection isn't available when trying to send an email, the following exception occurs after the timeout:

org.simplejavamail.mailer.internal.MailerException: Failed to send email [Subject: 'test'], reason: Unknown error
	at org.simplejavamail.mailer.internal.SendMailClosure.handleException(SendMailClosure.java:86)
	at org.simplejavamail.mailer.internal.SendMailClosure.executeClosure(SendMailClosure.java:77)
	at org.simplejavamail.mailer.internal.AbstractProxyServerSyncingClosure.run(AbstractProxyServerSyncingClosure.java:56)
	at org.simplejavamail.mailer.internal.MailerImpl.sendMail(MailerImpl.java:361)
	at org.simplejavamail.mailer.internal.MailerImpl.sendMail(MailerImpl.java:347)
	...

Caused by: java.lang.NullPointerException: Cannot invoke "org.bbottema.genericobjectpool.PoolableObject.invalidate()" because "this.pooledTransport" is null
	at org.simplejavamail.internal.batchsupport.LifecycleDelegatingTransportImpl.signalTransportFailed(LifecycleDelegatingTransportImpl.java:55)
	at org.simplejavamail.mailer.internal.util.TransportRunner.sendUsingConnectionPool(TransportRunner.java:87)
	at org.simplejavamail.mailer.internal.util.TransportRunner.runOnSessionTransport(TransportRunner.java:69)
	at org.simplejavamail.mailer.internal.util.TransportRunner.sendMessage(TransportRunner.java:51)
	at org.simplejavamail.mailer.internal.SendMailClosure.executeClosure(SendMailClosure.java:70)
	...

Which isn't very explicit about what the problem was.

This line in BatchSupport doesn't check if a connection could be claimed, which is then used in TransportRunner and causes a NPE, which is caught and causes another NPE at delegatingTransport.signalTransportFailed().

Maybe an exception could be thrown if pooledTransport is null after trying to claim it from the pool?

Btw, I made a Groovy script that uses the com.github.davidmoten:subethasmtp library to implement a local SMTP server and make testing the timeouts easier:

https://gist.github.com/lopardo/3348e086648c86494fa69078af45aea5

It can be run with groovy SmtpServer.groovy or probably from an IDE like IntelliJ IDEA without installing groovy.

The Simple Java Mail project uses the Wiser (subethasmtp) for live junit tests, btw.

This line in BatchSupport doesn't check if a connection could be claimed

Good catch! I'll have a look.

I've released 7.8.1 which now fails fast. Cheers!