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!