fix(mail-connector): check htmlBody and/or body is null and throw an error #3796
Conversation
…error if not throw an error, connector will retry it over and over again.
| if (smtpSendEmail.body() == null) { | ||
| throw new MessagingException("Text Body is null"); | ||
| } | ||
| MimeBodyPart textPart = new MimeBodyPart(); | ||
| textPart.setText(smtpSendEmail.body(), StandardCharsets.UTF_8.name()); | ||
| multipart.addBodyPart(textPart); | ||
| } | ||
| case HTML -> { | ||
| if (smtpSendEmail.htmlBody() == null) { | ||
| throw new MessagingException("HTML Body is null"); | ||
| } | ||
| MimeBodyPart htmlPart = new MimeBodyPart(); | ||
| htmlPart.setContent(smtpSendEmail.htmlBody(), JakartaUtils.HTML_CHARSET); | ||
| multipart.addBodyPart(htmlPart); | ||
| } | ||
| case MULTIPART -> { | ||
| if (smtpSendEmail.htmlBody() == null || smtpSendEmail.body() == null) { | ||
| throw new MessagingException("Text or HTML Body is null"); | ||
| } |
There was a problem hiding this comment.
Hey @itsnuyen, instead of throwing a MessagingException, could you implement a custom validation method in the model that verifies that these values are not null?
There was a problem hiding this comment.
Hi @sbuettner,
I have one questions. Do you mean adapting inside the SmtpSendEmail record the validation like this?
switch (contentType) {
case PLAIN:
if (body == null) {
throw new IllegalArgumentException("Email body is null");
}
case HTML:
if (htmlBody == null) {
throw new IllegalArgumentException("Email html body is null");
}
case MULTIPART:
if (body == null || htmlBody == null) {
throw new IllegalArgumentException("Email body or html body cannot be null");
}
}This could also running but as a error message it could be confusing for the end user:
As it state that an exception is raised and it is a JSON_PROCESSING_ERROR
Do you have an Idea how I could provide the end user proper feedback?
Or do I even misunderstand your request?
There was a problem hiding this comment.
Hey @itsnuyen I was thinking about something like this as part of the SmtpSendEmail record:
@AssertTrue(message = "Please provide a proper message body")
public boolean isValidMessageBody() {
return body != null || htmlBody != null;
}
|
Great work @itsnuyen, could you add a simple test that covers this new validation? |
…nnector' into pham-minh-nguyen-fix-smtp-mailconnector
There was a problem hiding this comment.
LGTM,
looks like the test is missing the right license information:
Warning: Missing header in: /home/runner/work/connectors/connectors/connectors/email/src/test/java/io/camunda/connector/email/outbound/protocols/actions/SmtpSendEmailTest.java
You can use the license plugin to make sure its added to all files.
|
LGTM, Great work 😄 |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release/8.6
git worktree add -d .worktree/backport-3796-to-release/8.6 origin/release/8.6
cd .worktree/backport-3796-to-release/8.6
git switch --create backport-3796-to-release/8.6
git cherry-pick -x 657810f4bb25bf2da74bd19c0e7906a95aa934a3 |
…error (#3796) * fix(mail-connector): check htmlBody and/or body is null and throw an error if not throw an error, connector will retry it over and over again. * chore(mail-connector): build the assertation inside the `SmtpSendMail` record * test(mail-connector): adding unit test to validate `isEmailMessageValid` * chore(mail-connector): run mvn spotless to format the `SmtpSendEmailTest` class * chore(mail-connector): use mvn:licence to add the licence to the file (cherry picked from commit 657810f)
…error (#3796) * fix(mail-connector): check htmlBody and/or body is null and throw an error if not throw an error, connector will retry it over and over again. * chore(mail-connector): build the assertation inside the `SmtpSendMail` record * test(mail-connector): adding unit test to validate `isEmailMessageValid` * chore(mail-connector): run mvn spotless to format the `SmtpSendEmailTest` class * chore(mail-connector): use mvn:licence to add the licence to the file (cherry picked from commit 657810f)
Description
Because FEEL will take a string and a data and try to sum it, it will produce a null value.
This null value will produce a nullpointer as an error but the process is still running and after the defined retries it will break.
If you have multiple process running, this behavior will block everything else
So check if the content of the body is null and throw a null pointer
Related issues
closes ##3795
Checklist
no milestonelabel.