Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

Handle attempt to send notification to user without e-mail address more gracefully

Description

Attempting to send an e-mail to a user that does not have an e-mail address set currently results in an exception both in the notification service and then consequently in the service that was trying to use the notification service. This should be handled more gracefully and translated into our own validation (should we even throw an exception in such case?)

Current stacktrace from notification service:

10:59:33.383 malawi-production /var/log/messages 2017-10-13T08:59:32+00:00 6472e5189e0a [nio-8080-exec-3] ERROR org.openlmis.notification.web.NotificationController Unable to send notification 10:59:33.383 malawi-production /var/log/messages 2017-10-13T08:59:32+00:00 6472e5189e0a #011java.lang.IllegalArgumentException: To address must not be null 10:59:33.384 malawi-production /var/log/messages 2017-10-13T08:59:32+00:00 6472e5189e0a #011at org.springframework.util.Assert.notNull(Assert.java:115) 10:59:33.384 malawi-production /var/log/messages 2017-10-13T08:59:32+00:00 6472e5189e0a #011at org.springframework.mail.javamail.MimeMessageHelper.setTo(MimeMessageHelper.java:585) 10:59:33.384 malawi-production /var/log/messages 2017-10-13T08:59:32+00:00 6472e5189e0a #011at org.openlmis.notification.service.NotificationService.sendNotification(NotificationService.java:50) 10:59:33.384 malawi-production /var/log/messages 2017-10-13T08:59:32+00:00 6472e5189e0a #011at org.openlmis.notification.web.NotificationController.sendNotification(NotificationController.java:51)

Requisition service trying to use Notification service:

09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 [o-8080-exec-211] ERROR org.openlmis.requisition.service.NotificationService Can not send notification 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011org.springframework.web.client.HttpServerErrorException: 500 null 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:94) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.springframework.web.client.RestTemplate.handleResponse(RestTemplate.java:667) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:620) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.springframework.web.client.RestTemplate.execute(RestTemplate.java:595) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.springframework.web.client.RestTemplate.postForObject(RestTemplate.java:398) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.openlmis.requisition.service.NotificationService.notify(NotificationService.java:61) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.openlmis.requisition.service.ConvertToOrderNotifier.notifyConvertToOrder(ConvertToOrderNotifier.java:87) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.openlmis.requisition.service.DefaultRequisitionStatusProcessor.statusChange(DefaultRequisitionStatusProcessor.java:46) 09:20:21.755 malawi-production /var/log/messages 2017-10-13T07:20:20+00:00 efaa66a2f3b9 #011at org.openlmis.requisition.service.RequisitionService.convertToOrder(RequisitionService.java:609)

Environment

None

Attachments

1
  • 31 Oct 2017, 07:27 AM

Confluence content

mentioned on

QAlity Plus - Test Management

Checklists

Activity

Joanna Bebak 
October 31, 2017 at 7:28 AM

I tested the ticket, and everything works correctly. The "Send reset email" button is disabled when the user does not have an email address. Also, no requests are made after clicking on it, which is visible in the "Network" tool. When one enables the button with the use of Inspector, the server returns a correct status, "400", and a translated error message occurs, as visible on the screenshot:

Josh Zamor 
October 26, 2017 at 3:34 PM
(edited)

Regarding the validator on subject and content fields, we'd like to stop the email from going out to the user. However we'd like to not throw an exception. Lets just put an ERROR in the log.

Contact and with questions.

Please add a "steps to reproduce" or a test plan and QA it with:

  • a person with no email address

Łukasz Lewczyński 
October 26, 2017 at 1:48 PM

It seems like most of our services handle exception when it is throw because notification request failed (one exception is in the auth service but it should not hard to change that). I added validator for notification request to check if from and to fields have values - because I am able to send an empty email/without subject and content. Do you think this kind of validator is what we need? Should we not allow to empty subject and content fields? Should I change the way services handle bad request from the notification service? Currently they simple log as an error message and execute the next step in the code.

cc:

Brandon Bowersox-Johnson 
October 25, 2017 at 10:08 PM

We do not want Notification service to throw an Exception here. Perhaps Notification service should return a 400 status (not a 200). The service that called Notification (ReferenceData in this case of editing a user), it should accept the 400 status and it should proceed with normal processing. An error in the Notification service should not be treated as a failure up-stream. Notification service might not even exist in some implementations... (some day).

Done
Pinned fields
Click on the next to a field label to start pinning.

Details

Assignee

Reporter

Story Points

Original estimate

Time tracking

1d 1h 30m logged6h 30m remaining

Components

Sprint

Fix versions

Affects versions

Priority

Time Assistant

Created October 13, 2017 at 10:52 AM
Updated November 13, 2017 at 6:54 PM
Resolved October 31, 2017 at 7:29 AM