CAMEL-24403: camel-mail: handle MessageRemovedException in processCommit to avoid null cause in error log

[mayurbm]

Summary

Fixes CAMEL-24403.

When an IMAP message is expunged between fetch time and the on-completion commit callback, IMAPMessage.setFlag() throws MessageRemovedException whose getMessage() returns null, producing a confusing log entry:

Caused by: [javax.mail.MessageRemovedException - null]

Root Cause

MessageRemovedException extends MessagingException but is constructed without a message string so getMessage() returns null. The existing catch (MessagingException e) block in processCommit() passed it directly to the exception handler, which included the null in the formatted output.

Fix

Catch MessageRemovedException before the generic MessagingException handler in processCommit() and wrap it with an explicit description:

} catch (MessageRemovedException e) {
    MessagingException wrapped = new MessagingException(
            "Message already removed/expunged on server (no flag update possible)", e);
    getExceptionHandler().handleException(
            "Error occurred during committing mail message: " + mail, exchange, wrapped);
} catch (MessagingException e) {
    getExceptionHandler().handleException(
            "Error occurred during committing mail message: " + mail, exchange, e);
}

Expected log output after fix:

Caused by: [javax.mail.MessagingException - Message already removed/expunged on server (no flag update possible)]

Stack Trace (from CAMEL-24403)

javax.mail.MessageRemovedException
  at com.sun.mail.imap.IMAPMessage.checkExpunged(IMAPMessage.java:280)
  at com.sun.mail.imap.IMAPMessage.setFlags(IMAPMessage.java:1110)
  at javax.mail.Message.setFlag(Message.java:596)
  at org.apache.camel.component.mail.MailConsumer.processCommit(MailConsumer.java:506)
  at org.apache.camel.component.mail.MailConsumer$1.onComplete(MailConsumer.java:237)
  at org.apache.camel.support.UnitOfWorkHelper.doneSynchronization(UnitOfWorkHelper.java:104)
  at org.apache.camel.support.UnitOfWorkHelper.doneSynchronizations(UnitOfWorkHelper.java:89)
  at org.apache.camel.impl.engine.DefaultUnitOfWork.done(DefaultUnitOfWork.java:238)
  at org.apache.camel.support.UnitOfWorkHelper.doneUow(UnitOfWorkHelper.java:61)
  at org.apache.camel.impl.engine.CamelInternalProcessor$UnitOfWorkProcessorAdvice.after(CamelInternalProcessor.java:770)
  at org.apache.camel.impl.engine.CamelInternalProcessor$AsyncAfterTask.done(CamelInternalProcessor.java:263)
  at org.apache.camel.AsyncCallback.run(AsyncCallback.java:44)
  at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.schedule(DefaultReactiveExecutor.java:193)
  at org.apache.camel.processor.Pipeline.process(Pipeline.java:185)
  at org.apache.camel.impl.engine.CamelInternalProcessor.process(CamelInternalProcessor.java:398)
  at org.apache.camel.component.mail.MailConsumer.processExchange(MailConsumer.java:449)
  at org.apache.camel.component.mail.MailConsumer.processBatch(MailConsumer.java:258)
  at org.apache.camel.component.mail.MailConsumer.poll(MailConsumer.java:165)
  at org.apache.camel.support.ScheduledPollConsumer.doRun(ScheduledPollConsumer.java:202)
  at org.apache.camel.support.ScheduledPollConsumer.run(ScheduledPollConsumer.java:116)
  at org.apache.camel.pollconsumer.quartz.QuartzScheduledPollConsumerJob.execute(QuartzScheduledPollConsumerJob.java:61)
  at org.quartz.core.JobRunShell.run(JobRunShell.java:202)
  at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:573)

Changes

• components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java — add jakarta.mail.MessageRemovedException import; split catch block
• components/camel-mail/src/test/java/org/apache/camel/component/mail/MailConsumerCommitExpungedMessageTest.java (new) — 2 unit tests:
  • testCommitWithExpungedMessageProducesNonNullCause — verifies MessageRemovedException is wrapped with a non-null message and root cause preserved
  • testCommitWithOtherMessagingExceptionPassedThroughAsIs — verifies other MessagingException types pass through unchanged

Test Results

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0  (JDK 21, Maven 3.9)

AI Attribution

This contribution was developed with AI assistance using Claude Code.

Co-authored-by: Claude <claude@anthropic.com>

[mayurbm]

JIRA Issue: CAMEL-24403

Link: https://issues.apache.org/jira/browse/CAMEL-24403
Summary: camel-mail: MessageRemovedException in processCommit causes null in error log
Type: Bug
Component: camel-mail

Problem

When an IMAP message is expunged (deleted by another client or server-side policy) between the time it is fetched by and when the on-completion commit callback fires, throws . This exception is constructed without a message string so returns , producing:

Stack Trace

Root Cause

extends but is constructed without a message string, so returns . The existing block in passed it directly to the exception handler, which included the null in the formatted output.

Expected Log After Fix

[atiaomar1978-hub]

Review summary

AI-generated review on behalf of atiaomar1978-hub

Focused, correct bug fix for CAMEL-24403. When an IMAP message is already expunged on the server, Message.setFlag() throws MessageRemovedException with a null message, which produced useless Caused by: [... - null] lines in the commit error log.

What looks good

• Dedicated catch (MessageRemovedException) before the generic MessagingException handler — correct, since MessageRemovedException extends MessagingException.
• Wrapping preserves the original exception as cause while supplying a clear operator-facing message.
• The catch covers the whole processCommit try block, so expunged messages during copy/move (copyOrMoveMessageIfRequired) get the same treatment — not just setFlag.
• Two targeted unit tests: expunged-message wrapping + regression that other MessagingExceptions pass through unchanged.
• Minimal production diff (6 lines) with meaningful test coverage.

Suggestions (non-blocking)

• New test class uses public visibility and JUnit assertions — project convention prefers package-private tests and AssertJ (assertThat). Minor polish only; existing camel-mail tests are mixed.

Verdict

Approve — ready to merge once CI is green.

---

Review performed with code inspection and Bugbot. Does not replace specialized tools (CodeRabbit, Sourcery, SonarCloud).

[atiaomar1978-hub]

Review complete ✅

AI-generated on behalf of atiaomar1978-hub

Verdict: Approve — clean fix for the null-cause logging issue.

Bugbot found no functional bugs. One minor convention note on the new test class (package-private + AssertJ) — non-blocking.

Note: CI checks were not yet reported on the branch at review time — please confirm green before merge.

Thanks @mayurbm for the focused fix and the regression test guarding the pass-through behaviour for other MessagingExceptions.

[mayurbm]

Addressed review feedback from @atiaomar1978-hub:

Comment 1 (MailConsumer.java line 530 — catch placement):
Acknowledged and agreed. The catch-before-MessagingException ordering is required and intentional. The wrapping also covers expunged messages during copy/move, not only setFlag — no code change needed.

Comment 2 (MailConsumerCommitExpungedMessageTest.java line 52 — test conventions):
Applied in commit cb2b59c:

• Dropped public from class and both test methods (now package-private)
• Replaced all JUnit assertEquals/assertInstanceOf/assertNotNull with AssertJ assertThat(...).isInstanceOf().hasMessage().hasCauseInstanceOf() chains

Local validation after changes:

• mvn formatter:format impsort:sort — no changes needed (already clean)
• Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 [JDK 21 / Maven 3.9]