From 547ea457372066456c0efc982eb49cd648b0e2fc Mon Sep 17 00:00:00 2001 From: Ryan Miles Date: Thu, 22 Jan 2026 13:49:20 -0800 Subject: [PATCH 1/5] Enhance exception handling to include detailed error message in system error response --- cwms-data-api/src/main/java/cwms/cda/ApiServlet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java index 87c1b1150..2eb6f177a 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -381,7 +381,7 @@ public void init() { ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(Exception.class, (e, ctx) -> { - CdaError errResponse = new CdaError("System Error"); + CdaError errResponse = new CdaError("System Error", Map.of("Error message", e.getMessage())); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); From 1fc2ecf5729113f807adb5ce4274daaf18d05213 Mon Sep 17 00:00:00 2001 From: Ryan Miles Date: Fri, 30 Jan 2026 17:19:37 -0800 Subject: [PATCH 2/5] Sanitizing DataAccessExceptions in the logger. --- .../src/main/java/cwms/cda/ApiServlet.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java index 2eb6f177a..ed66c32b3 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -194,6 +194,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.sql.SQLException; import java.time.DateTimeException; import java.util.ArrayList; import java.util.Arrays; @@ -212,6 +213,7 @@ import javax.sql.DataSource; import org.apache.http.entity.ContentType; import org.jetbrains.annotations.NotNull; +import org.jooq.exception.DataAccessException; import org.owasp.html.HtmlPolicyBuilder; import org.owasp.html.PolicyFactory; @@ -380,6 +382,15 @@ public void init() { CdaError re = new CdaError(e.getMessage()); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) + .exception(DataAccessException.class, (e, ctx) -> { + String message = getSanitizedDataAccessException(e); + CdaError errResponse = new CdaError("System Error", Map.of("Error message", message)); + logger.atWarning().withCause(e).log("error on request[%s]: %s", + errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); + ctx.status(500); + ctx.contentType(ContentType.APPLICATION_JSON.toString()); + ctx.json(errResponse); + }) .exception(Exception.class, (e, ctx) -> { CdaError errResponse = new CdaError("System Error", Map.of("Error message", e.getMessage())); logger.atWarning().withCause(e).log("error on request[%s]: %s", @@ -400,6 +411,21 @@ public void init() { logger.atInfo().log("Javalin initialized."); } + private static String getSanitizedDataAccessException(DataAccessException e) { + Throwable cause = e.getCause(); + String message = ""; + if (cause instanceof SQLException) { + String localizedMessage = cause.getLocalizedMessage(); + String[] parts = localizedMessage.split("\n"); + message = parts[0]; + int index = message.indexOf(":"); + if (index >= 0) { + message = message.substring(index + 1); + } + } + return message; + } + private String obtainFullVersion(ServletConfig servletConfig) throws ServletException { String relativeWARPath = "/META-INF/MANIFEST.MF"; String absoluteDiskPath = servletConfig.getServletContext().getRealPath(relativeWARPath); From 74da8dd64172f51e95d55839e80d199efaebd97c Mon Sep 17 00:00:00 2001 From: rma-rripken <89810919+rma-rripken@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:11:52 -0800 Subject: [PATCH 3/5] Removed code from ApiServlet that would send users unknown exception messages and handling Rate related error in RateDao instead. --- .../src/main/java/cwms/cda/ApiServlet.java | 33 ++++------ .../cda/api/errors/NoDataRateException.java | 21 +++++++ .../cwms/cda/api/errors/RateException.java | 13 +++- .../main/java/cwms/cda/data/dao/RateDao.java | 60 ++++++++++++------- 4 files changed, 83 insertions(+), 44 deletions(-) create mode 100644 cwms-data-api/src/main/java/cwms/cda/api/errors/NoDataRateException.java diff --git a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java index ed66c32b3..000bac9a5 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -194,7 +194,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.sql.SQLException; import java.time.DateTimeException; import java.util.ArrayList; import java.util.Arrays; @@ -272,7 +271,7 @@ public class ApiServlet extends HttpServlet { public static final String OFFICE_ID = "office_id"; public static final String DATA_SOURCE = "data_source"; public static final String RAW_DATA_SOURCE = "data_source"; - public static final String DATABASE = "database"; + public static final String IS_NEW_LRTS = "X-CWMS-LRTS-Formatting"; // The VERSION should match the gradle version but not contain the patch version. @@ -383,8 +382,17 @@ public void init() { ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DataAccessException.class, (e, ctx) -> { - String message = getSanitizedDataAccessException(e); - CdaError errResponse = new CdaError("System Error", Map.of("Error message", message)); + // Whatever Dao is causing this exception to be thrown should be modified. + // The preferred pattern is for the Dao to catch DataAccessExceptions exceptions + // and for the dao to inspect the Oracle error code or error message as necessary + // to transform DataAccessExceptions (and their SQLException causes) + // into specific and appropriate exceptions with + // messages that are helpful and meaningful to end-users. + + // CdaError does not include the Oracle exception message b/c this block catches + // all unhandled DataAccessExceptions and we don't know what is in the message + // it is unknown if the message would be safe/appropriate for users to see. + CdaError errResponse = new CdaError("Database Error"); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); @@ -392,7 +400,7 @@ public void init() { ctx.json(errResponse); }) .exception(Exception.class, (e, ctx) -> { - CdaError errResponse = new CdaError("System Error", Map.of("Error message", e.getMessage())); + CdaError errResponse = new CdaError("System Error"); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); @@ -411,21 +419,6 @@ public void init() { logger.atInfo().log("Javalin initialized."); } - private static String getSanitizedDataAccessException(DataAccessException e) { - Throwable cause = e.getCause(); - String message = ""; - if (cause instanceof SQLException) { - String localizedMessage = cause.getLocalizedMessage(); - String[] parts = localizedMessage.split("\n"); - message = parts[0]; - int index = message.indexOf(":"); - if (index >= 0) { - message = message.substring(index + 1); - } - } - return message; - } - private String obtainFullVersion(ServletConfig servletConfig) throws ServletException { String relativeWARPath = "/META-INF/MANIFEST.MF"; String absoluteDiskPath = servletConfig.getServletContext().getRealPath(relativeWARPath); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/errors/NoDataRateException.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/NoDataRateException.java new file mode 100644 index 000000000..1be35d52e --- /dev/null +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/NoDataRateException.java @@ -0,0 +1,21 @@ +package cwms.cda.api.errors; + +import java.sql.SQLException; +import java.util.LinkedHashMap; +import java.util.logging.Level; + +public class NoDataRateException extends RateException{ + // 404 is Not Found - this isn't quite right b/c the rate() or reverse-rate() isn't failing b/c + // the ts isn't found its failing b/c a necessary rating table isn't found. + // 424 is Failed Dependency - this is closer to what we want, but it's not quite right either. + + // 422 is Unprocessable Entity. I think this is the closest. + + public static final int HTTP_ERROR_CODE = 422; + + public NoDataRateException(String message, SQLException cause) { + super(message, DATABASE_SOURCE, "Error performing rate function: " + message, + HTTP_ERROR_CODE, Level.INFO, new LinkedHashMap<>(), cause); + } + +} diff --git a/cwms-data-api/src/main/java/cwms/cda/api/errors/RateException.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/RateException.java index 6e0c3f009..69504204c 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/errors/RateException.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/RateException.java @@ -24,16 +24,23 @@ package cwms.cda.api.errors; +import java.io.Serializable; import java.sql.SQLException; -import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.logging.Level; import javax.servlet.http.HttpServletResponse; -public final class RateException extends ApplicationException { +public class RateException extends ApplicationException { private static final Level LOG_LEVEL = Level.INFO; + public RateException(String message, String source, String cdaErrorMessage, int cdaHttpErrorCode, + Level logLevel, Map details, Throwable cause) { + super(message, source, cdaErrorMessage, cdaHttpErrorCode, logLevel, details, cause); + } + public RateException(String message, SQLException cause) { super(message, DATABASE_SOURCE, "Error performing rate function: " + message, - HttpServletResponse.SC_INTERNAL_SERVER_ERROR, LOG_LEVEL, new HashMap<>(), cause); + HttpServletResponse.SC_INTERNAL_SERVER_ERROR, LOG_LEVEL, new LinkedHashMap<>(), cause); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java index ca22b869c..1ae74b439 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java @@ -26,6 +26,7 @@ import static java.util.stream.Collectors.toList; +import cwms.cda.api.errors.NoDataRateException; import cwms.cda.api.errors.RateException; import cwms.cda.data.dto.CwmsId; import cwms.cda.data.dto.TimeSeries; @@ -44,6 +45,7 @@ import org.jooq.ConnectionCallable; import org.jooq.DSLContext; import org.jooq.exception.DataAccessException; +import org.jspecify.annotations.Nullable; import usace.cwms.db.jooq.codegen.packages.CWMS_RATING_PACKAGE; import usace.cwms.db.jooq.codegen.udt.records.DATE_TABLE_TYPE; import usace.cwms.db.jooq.codegen.udt.records.DOUBLE_TAB_T; @@ -59,22 +61,22 @@ public RateDao(DSLContext dsl) { } public RatedOutput rate(String officeId, String ratingId, RateInputValues input) { - DOUBLE_TAB_T outputValues = connectionResult(c -> { - DSLContext context = getDslContext(c, officeId); - DATE_TABLE_TYPE ratingDates = null; - if (!input.getValueTimes().isEmpty()) { + DOUBLE_TAB_T outputValues = connectionResult(c -> { + DSLContext context = getDslContext(c, officeId); + DATE_TABLE_TYPE ratingDates = null; + if (!input.getValueTimes().isEmpty()) { - ratingDates = new DATE_TABLE_TYPE(); - input.getValueTimes().stream().map(Timestamp::new).forEach(ratingDates::add); - } - DOUBLE_TAB_TAB_T inputValues = new DOUBLE_TAB_TAB_T(); - input.getValues().stream().map(DOUBLE_TAB_T::new).forEach(inputValues::add); - STR_TAB_T unitsTab = new STR_TAB_T(input.getInputUnits()); - unitsTab.add(input.getOutputUnit()); - return CWMS_RATING_PACKAGE.call_RATE(context.configuration(), ratingId, - inputValues, unitsTab, formatBool(input.getRound()), ratingDates, null, "UTC", officeId); - }); - return new RatedOutputValues(CwmsId.buildCwmsId(officeId, ratingId), outputValues, input.getOutputUnit()); + ratingDates = new DATE_TABLE_TYPE(); + input.getValueTimes().stream().map(Timestamp::new).forEach(ratingDates::add); + } + DOUBLE_TAB_TAB_T inputValues = new DOUBLE_TAB_TAB_T(); + input.getValues().stream().map(DOUBLE_TAB_T::new).forEach(inputValues::add); + STR_TAB_T unitsTab = new STR_TAB_T(input.getInputUnits()); + unitsTab.add(input.getOutputUnit()); + return CWMS_RATING_PACKAGE.call_RATE(context.configuration(), ratingId, + inputValues, unitsTab, formatBool(input.getRound()), ratingDates, null, "UTC", officeId); + }); + return new RatedOutputValues(CwmsId.buildCwmsId(officeId, ratingId), outputValues, input.getOutputUnit()); } private void validateReverseRateInput(RateInput input) { @@ -172,15 +174,31 @@ static RuntimeException handleRateDbError(DataAccessException ex) { if (cause instanceof SQLException) { int errorCode = ((SQLException) cause).getErrorCode(); if (errorCode == 20019 || errorCode == 20998) { - String localizedMessage = cause.getLocalizedMessage(); - String[] parts = localizedMessage.split("\n"); - String message = parts[0]; - int index = message.indexOf(":"); - if (index >= 0) { - retval = new RateException(message.substring(index + 1), (SQLException) cause); + String message = getMessage(cause); + if( message != null ) { + retval = new RateException(message, (SQLException) cause); + } + } else if (errorCode == 1403){ + String message = getMessage(cause); + if( message != null ) { + // firstMessage may be like "no data found" + // or "Table rating has no rating points" + retval = new NoDataRateException(message, (SQLException) cause); } } } return retval; } + + private static @Nullable String getMessage(Throwable cause) { + String firstMessage = null; + String localizedMessage = cause.getLocalizedMessage(); + String[] parts = localizedMessage.split("\n"); + String message = parts[0]; + int index = message.indexOf(":"); + if (index >= 0) { + firstMessage = message.substring(index + 1); + } + return firstMessage; + } } From 9ecc885ad244a191eca1dd078109aff08d3f04ce Mon Sep 17 00:00:00 2001 From: rma-rripken <89810919+rma-rripken@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:21:36 -0800 Subject: [PATCH 4/5] Fixed indentation --- .../main/java/cwms/cda/data/dao/RateDao.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java index 1ae74b439..1b45b8564 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/RateDao.java @@ -61,22 +61,22 @@ public RateDao(DSLContext dsl) { } public RatedOutput rate(String officeId, String ratingId, RateInputValues input) { - DOUBLE_TAB_T outputValues = connectionResult(c -> { - DSLContext context = getDslContext(c, officeId); - DATE_TABLE_TYPE ratingDates = null; - if (!input.getValueTimes().isEmpty()) { + DOUBLE_TAB_T outputValues = connectionResult(c -> { + DSLContext context = getDslContext(c, officeId); + DATE_TABLE_TYPE ratingDates = null; + if (!input.getValueTimes().isEmpty()) { - ratingDates = new DATE_TABLE_TYPE(); - input.getValueTimes().stream().map(Timestamp::new).forEach(ratingDates::add); - } - DOUBLE_TAB_TAB_T inputValues = new DOUBLE_TAB_TAB_T(); - input.getValues().stream().map(DOUBLE_TAB_T::new).forEach(inputValues::add); - STR_TAB_T unitsTab = new STR_TAB_T(input.getInputUnits()); - unitsTab.add(input.getOutputUnit()); - return CWMS_RATING_PACKAGE.call_RATE(context.configuration(), ratingId, - inputValues, unitsTab, formatBool(input.getRound()), ratingDates, null, "UTC", officeId); - }); - return new RatedOutputValues(CwmsId.buildCwmsId(officeId, ratingId), outputValues, input.getOutputUnit()); + ratingDates = new DATE_TABLE_TYPE(); + input.getValueTimes().stream().map(Timestamp::new).forEach(ratingDates::add); + } + DOUBLE_TAB_TAB_T inputValues = new DOUBLE_TAB_TAB_T(); + input.getValues().stream().map(DOUBLE_TAB_T::new).forEach(inputValues::add); + STR_TAB_T unitsTab = new STR_TAB_T(input.getInputUnits()); + unitsTab.add(input.getOutputUnit()); + return CWMS_RATING_PACKAGE.call_RATE(context.configuration(), ratingId, + inputValues, unitsTab, formatBool(input.getRound()), ratingDates, null, "UTC", officeId); + }); + return new RatedOutputValues(CwmsId.buildCwmsId(officeId, ratingId), outputValues, input.getOutputUnit()); } private void validateReverseRateInput(RateInput input) { From f11f64143c42116d9a1291b534f074b0938d3b07 Mon Sep 17 00:00:00 2001 From: rma-rripken <89810919+rma-rripken@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:28:07 -0800 Subject: [PATCH 5/5] Restoring variable. --- cwms-data-api/src/main/java/cwms/cda/ApiServlet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java index 000bac9a5..37af31686 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -271,7 +271,7 @@ public class ApiServlet extends HttpServlet { public static final String OFFICE_ID = "office_id"; public static final String DATA_SOURCE = "data_source"; public static final String RAW_DATA_SOURCE = "data_source"; - + public static final String DATABASE = "database"; public static final String IS_NEW_LRTS = "X-CWMS-LRTS-Formatting"; // The VERSION should match the gradle version but not contain the patch version.