Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Closed
lopardo opened this issue Jan 26, 2023 · 4 comments

Comments

@lopardo
Copy link

lopardo commented Jan 26, 2023

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?

@lopardo
Copy link
Author

lopardo commented Jan 26, 2023

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.

@bbottema
Copy link
Owner

bbottema commented Feb 1, 2023

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

@bbottema
Copy link
Owner

bbottema commented Feb 1, 2023

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

Good catch! I'll have a look.

@bbottema bbottema changed the title Improve error handling when claiming a connection from the pool times out Bug: properly Fail-Fast in case of Transport claim timeout in the batch-module, rather than running into NPE further down the line Feb 1, 2023
bbottema added a commit that referenced this issue Feb 1, 2023
…. Otherwise, it becomes an NPE further down the chain.
@bbottema bbottema added this to the 7.8.1 milestone Feb 1, 2023
@bbottema
Copy link
Owner

bbottema commented Feb 1, 2023

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

@bbottema bbottema closed this as completed Feb 1, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants