Repository navigation
Description
so i was going through the controller layer in HWC-API trying to understand how the API responses work, and i noticed something that feels off.
every controller method returns String instead of ResponseEntity. which means the actual HTTP status is always 200, whether the request worked or failed. the error details are just stuffed inside the JSON body.
here's what it looks like in GeneralOPDController.java:
public String saveBenGenOPDNurseData(@RequestBody String requestObj, ...) {
OutputResponse response = new OutputResponse();
try {
response.setResponse(genOPDRes);
} catch (Exception e) {
response.setError(5000, "Unable to save data");
}
return response.toString(); // always 200
}this same pattern is in every controller i checked,generalOPD, ncdscreening, cancerscreening, all of them. the client always gets 200 and has to dig into the body to figure out if things actually failed.the funny part is, OutputResponse.java already has a method called toStringWithHttpStatus() that returns proper ResponseEntity with correct status codes. but none of the controllers use it, they all call toString() instead.
also noticed a couple of things in OutputResponse.java while i was there:
- BAD_REQUEST is set to 404, but 404 is Not Found. Bad Request is 400
- SWYMED_EXCEPTION and TM_EXCEPTION both use error code 5010 so theres no way to tell them apart the other thing i noticed,and this one feels more important given that AMRIT is a healthcare platform,is that every controller logs the full raw request body at INFO level:
logger.info("Request object for GeneralOPD nurse data saving :" + requestObj);these request bodies have patient vitals, medical history, examination details, prescriptions. INFO level logs are usually always on in production, so all this patient data is probably sitting in log files or getting shipped to whatever log aggregator they use.
both of these come from the same root issue honestly,heres no @ControllerAdvice or any kind of centralized exception handling. every controller method has its own try-catch doing the exact same thing. thats why the pattern is so consistent across all 20+ controllers.
i'd like to take a shot at fixing this. my plan would be:
- add a @ControllerAdvice global exception handler
- refactor one controller (probably GeneralOPDController) to use ResponseEntity and the existing toStringWithHttpStatus() method as a proof of concept
- remove the raw request body logging and replace it with something that doesnt dump full patient data
not trying to rewrite everything in one go, just start with one controller and see if the approach works.
happy to get reviews and replies???
Activity
ayushk687 commentedon May 12, 2026
@drtechie please assign this iisue to me.
sharma-sugurthi commentedon May 18, 2026
hey,I opened this issue and deliberately held off on submitting a PR because this isn't a simple bug fix. changing every controller's return type from String to ResponseEntity affects the API contract for every consumer,the frontend (HttpInterceptorService in HWC-UI), any mobile clients, and any external integrations. if we suddenly start returning HTTP 400/500 instead of 200-with-error-in-body, existing clients that check response.statusCode from the JSON body will break.
that's a core architectural decision that needs maintainer input before anyone starts writing code. specifically:
I was waiting for @drtechie's input on these questions before submitting a PR.rushing a fix without that alignment risks breaking production clients across 16 states.
sharma-sugurthi commentedon May 18, 2026
but I also reviewed the PRs that came in:
#211 - JSON parsing (JsonParser/getAsJsonObject()) is outside the try-catch,so malformed requests throw unhandled JsonSyntaxException -> raw 500 instead of structured error. 0% test coverage on new code.
#213- more comprehensive, but CodeRabbit caught that failure branches still call setResponse(...) instead of setError(...), so those paths still return HTTP 200,the exact bug this issue describes. the GlobalExceptionHandler also echoes ex.getMessage() into the response body via setError(ex), which leaks internal details. SonarCloud failed the quality gate (3.1% duplication > 3%).
neither PR addresses the frontend compatibility concern, which is the actual hard part of this fix.
sharma-sugurthi commentedon May 18, 2026
once there's alignment on the migration approach, I'll submit a PR that covers the HTTP status fix, PII logging fix, and error code corrections,with tests and frontend impact documented.
@drtechie needs your cosent on this decision...!