Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

RA-1889:Return Relatively Formatted Error Instead Of Returning Full Exception #43

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

RA-1889:Return Relatively Formatted Error Instead Of Returning Full Exception #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
37aa36a
RA-1889:Return Relativively Formatted Error Message
sherrif10 Feb 10, 2021
6c7e64a
Collecting the exception handler
sherrif10 Feb 10, 2021
64ffe44
adding error view page
sherrif10 Feb 10, 2021
3302488
Using ArrayList
sherrif10 Feb 10, 2021
85a1720
Collecting the error message
sherrif10 Feb 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion ...ule/htmlformentryui/page/controller/htmlform/ViewEncounterWithHtmlFormPageController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
package org.openmrs.module.htmlformentryui.page.controller.htmlform;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import java.util.List;

import org.apache.commons.lang.StringUtils;
import org.openmrs.Encounter;
import org.openmrs.api.AdministrationService;
import org.openmrs.messagesource.MessageSourceService;
import org.openmrs.module.emrapi.patient.PatientDomainWrapper;
import org.openmrs.module.htmlformentry.HtmlForm;
import org.openmrs.module.htmlformentry.HtmlFormEntryService;
Expand All @@ -11,9 +17,18 @@
import org.openmrs.ui.framework.annotation.InjectBeans;
import org.openmrs.ui.framework.annotation.SpringBean;
import org.openmrs.ui.framework.page.PageModel;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RequestParam;

public class ViewEncounterWithHtmlFormPageController {

private static final Log log = LogFactory.getLog(ViewEncounterWithHtmlFormPageController.class);

@Autowired
private MessageSourceService messageSourceService;

private String message;

public void get(@RequestParam("encounter") Encounter encounter,
@RequestParam(value = "showPatientHeader", defaultValue = "true") boolean showPatientHeader,
Expand All @@ -24,6 +39,7 @@ public void get(@RequestParam("encounter") Encounter encounter,
@SpringBean("htmlFormEntryService") HtmlFormEntryService htmlFormEntryService,
@SpringBean("adminService") AdministrationService administrationService,
UiUtils ui,
List<String> errors,
PageModel model) {

patient.setPatient(encounter.getPatient());
Expand Down Expand Up @@ -52,8 +68,10 @@ public void get(@RequestParam("encounter") Encounter encounter,
model.addAttribute("showPatientHeader", showPatientHeader);

HtmlForm htmlForm = htmlFormEntryService.getHtmlFormByForm(encounter.getForm());

if (htmlForm == null) {
throw new IllegalArgumentException("encounter.form is not an HTML Form: " + encounter.getForm());
log.warn("encounter.form is not an HTML Form");
errors.add("encounter.form is not an HTML Form:"+encounter.getForm());
}
model.addAttribute("htmlForm", htmlForm);
}
Expand All @@ -65,5 +83,12 @@ public void get(@RequestParam("encounter") Encounter encounter,
private String fixCase(String word) {
return Character.toUpperCase(word.charAt(0)) + word.substring(1).toLowerCase();
}

@ExceptionHandler(value= IllegalArgumentException.class)
public String HandleIllegalArgumentException(Exception e) {
log.warn("encounter.form is not an HTML Form");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this catch any Null Pointer Exception? How do we know that the "encounter.form" NPE was the one that caused it?

Copy link
Member Author

@sherrif10 sherrif10 Feb 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have changed it to illegalArgumentException, this method would handle any exception that would occur when any api with in the controller class returns an exception is called

return "IllegalArgumentException";
}


}
12 changes: 12 additions & 0 deletions omod/src/main/webapp/pages/htmlform/IllegalArgumentException.gsp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<html>
<body>
<p>Application has encountered an error. Please contact support on ...</p>
<% out << "encounter.form is not an HTML Form" %>
Copy link

@HerbertYiga HerbertYiga Feb 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the error you are returning? down your controller ,add the error message to an attribute for example errorMessage and access it from the gsp as ${errorMessage }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @HerbertYiga , ,Am not well convinced with this approach and am sure there is a clean way of doing this which am looking forward too.

<!--
Failed URL: ${url}
Exception: ${exception.message}
<c:forEach items="${exception.stackTrace}" var="ste"> ${ste}
</c:forEach>
-->
</body>
</html>