From b3593701ce16ef92afdb067c933169ab1734ebb0 Mon Sep 17 00:00:00 2001 From: Charles Graham Date: Wed, 8 Apr 2026 21:26:14 -0500 Subject: [PATCH 01/10] Create ErrorTraceSupport class to handle sending back stacktraces via response --- .../src/main/java/cwms/cda/ApiServlet.java | 18 +- .../cda/api/errors/ErrorTraceSupport.java | 161 ++++++++++++++++++ .../cda/api/errors/ErrorTraceSupportTest.java | 124 ++++++++++++++ 3 files changed, 295 insertions(+), 8 deletions(-) create mode 100644 cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java create mode 100644 cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.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 34207341f6..8d26760a65 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -106,6 +106,7 @@ import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.ApplicationException; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.api.location.kind.GateChangeCreateController; import cwms.cda.api.location.kind.GateChangeDeleteController; import cwms.cda.api.location.kind.GateChangeGetAllController; @@ -355,31 +356,32 @@ public void init() { ctx.header("X-XSS-Protection", "1; mode=block"); }) .exception(ApplicationException.class, (e, ctx) -> { - CdaError re = new CdaError(e.getCdaErrorMessage(), e.getSource(), e.getDetails()); + CdaError re = ErrorTraceSupport.buildError(ctx, e.getCdaErrorMessage(), + e.getSource(), e.getDetails(), e); if (e.getLoggerLevel().isPresent()) { logger.at(e.getLoggerLevel().get()).withCause(e).log(re.toString()); } ctx.status(e.getCdaHttpErrorCode()).json(re); }) .exception(UnsupportedOperationException.class, (e, ctx) -> { - final CdaError re = CdaError.notImplemented(); + final CdaError re = ErrorTraceSupport.buildError(ctx, "Not Implemented", e); logger.atWarning().withCause(e) .log("%s for request: %s", re, ctx.fullUrl()); ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(re); }) .exception(BadRequestResponse.class, (e, ctx) -> { - CdaError re = new CdaError("Bad Request", - "User Input", new HashMap<>(e.getDetails())); + CdaError re = ErrorTraceSupport.buildError(ctx, "Bad Request", + "User Input", new HashMap<>(e.getDetails()), e); logger.atInfo().withCause(e).log(re.toString()); ctx.status(e.getStatus()).json(re); }) .exception(IllegalArgumentException.class, (e, ctx) -> { - CdaError re = new CdaError("Bad Request"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Bad Request", e); logger.atInfo().withCause(e).log(re.toString()); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DateTimeException.class, (e, ctx) -> { - CdaError re = new CdaError(e.getMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, e.getMessage(), e); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DataAccessException.class, (e, ctx) -> { @@ -393,7 +395,7 @@ public void init() { // 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"); + CdaError errResponse = ErrorTraceSupport.buildError(ctx, "Database Error", e); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); @@ -401,7 +403,7 @@ public void init() { ctx.json(errResponse); }) .exception(Exception.class, (e, ctx) -> { - CdaError errResponse = new CdaError("System Error"); + CdaError errResponse = ErrorTraceSupport.buildError(ctx, "System Error", e); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java new file mode 100644 index 0000000000..fbc7525e47 --- /dev/null +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java @@ -0,0 +1,161 @@ +package cwms.cda.api.errors; + +import cwms.cda.data.dao.AuthDao; +import cwms.cda.security.DataApiPrincipal; +import cwms.cda.security.Role; +import cwms.cda.spi.IdentityProvider; +import io.javalin.http.Context; +import java.io.PrintWriter; +import java.io.Serializable; +import java.io.StringWriter; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +public final class ErrorTraceSupport { + public static final String STACK_TRACE_KEY = "stackTrace"; + static final String ALWAYS_SHOW_STACK_TRACE_PROPERTY = "cwms.dataapi.errors.alwaysShowStackTrace"; + static final String ALWAYS_SHOW_STACK_TRACE_VARIABLE = "CWMS_DATAAPI_ERRORS_ALWAYS_SHOW_STACK_TRACE"; + static final String PRIMARY_ENVIRONMENT_PROPERTY = "cwms.dataapi.environment.name"; + static final String PRIMARY_ENVIRONMENT_VARIABLE = "CWMS_DATAAPI_ENVIRONMENT_NAME"; + static final String HOST_ENVIRONMENT_VARIABLE = "ENVIRONMENT"; + static final String LEGACY_ENVIRONMENT_PROPERTY = "cda.environment.name"; + static final String LEGACY_ENVIRONMENT_VARIABLE = "CDA_ENVIRONMENT_NAME"; + static final String WAR_CONTEXT_PROPERTY = "warContext"; + private static final String DEV_MARKER = "dev"; + private static final Role CWMS_USER_ADMINS_ROLE = new Role("CWMS User Admins"); + + private ErrorTraceSupport() { + } + + public static CdaError buildError(Context ctx, String message, Throwable cause) { + Map details = buildDetails(ctx, Collections.emptyMap(), cause); + if (details.isEmpty()) { + return new CdaError(message); + } + return new CdaError(message, details); + } + + public static CdaError buildError(Context ctx, String message, String source, + Map details, Throwable cause) { + return new CdaError(message, source, buildDetails(ctx, details, cause)); + } + + static Map buildDetails(Context ctx, Map details, + Throwable cause) { + return buildDetails(details, cause, shouldIncludeStackTrace(ctx)); + } + + static Map buildDetails(Map details, + Throwable cause, boolean includeStackTrace) { + Map merged = new HashMap<>(); + if (details != null) { + merged.putAll(details); + } + if (cause != null && includeStackTrace) { + merged.put(STACK_TRACE_KEY, stackTraceOf(cause)); + } + return Collections.unmodifiableMap(merged); + } + + static boolean shouldIncludeStackTrace(Context ctx) { + if (alwaysShowStackTraceOverrideEnabled()) { + return true; + } + if (localhostRequestOverrideEnabled(ctx)) { + return true; + } + return shouldIncludeStackTrace(resolvePrincipal(ctx).orElse(null), resolveEnvironmentName(ctx)); + } + + static boolean shouldIncludeStackTrace(DataApiPrincipal principal, String environmentName) { + if (alwaysShowStackTraceOverrideEnabled()) { + return true; + } + return hasAdminRole(principal) && environmentLooksLikeDev(environmentName); + } + + static boolean alwaysShowStackTraceOverrideEnabled() { + return Boolean.parseBoolean(firstNonBlank( + System.getProperty(ALWAYS_SHOW_STACK_TRACE_PROPERTY), + System.getenv(ALWAYS_SHOW_STACK_TRACE_VARIABLE), + "false" + )); + } + + static boolean localhostRequestOverrideEnabled(Context ctx) { + if (ctx == null || ctx.req == null) { + return false; + } + return localhostRequestOverrideEnabled(ctx.req.getServerName()); + } + + static boolean localhostRequestOverrideEnabled(String serverName) { + return serverName != null + && ("localhost".equalsIgnoreCase(serverName) + || "127.0.0.1".equals(serverName) + || "::1".equals(serverName)); + } + + static Optional resolvePrincipal(Context ctx) { + if (ctx == null) { + return Optional.empty(); + } + DataApiPrincipal attributePrincipal = ctx.attribute(AuthDao.DATA_API_PRINCIPAL); + if (attributePrincipal != null) { + return Optional.of(attributePrincipal); + } + DataApiPrincipal sessionPrincipal = ctx.sessionAttribute(IdentityProvider.PRINCIPAL_KEY); + return Optional.ofNullable(sessionPrincipal); + } + + static boolean hasAdminRole(DataApiPrincipal principal) { + return principal != null + && principal.getRoles().contains(CWMS_USER_ADMINS_ROLE); + } + + static String resolveEnvironmentName(Context ctx) { + String configuredName = firstNonBlank( + System.getProperty(PRIMARY_ENVIRONMENT_PROPERTY), + System.getenv(PRIMARY_ENVIRONMENT_VARIABLE), + System.getenv(HOST_ENVIRONMENT_VARIABLE), + System.getProperty(LEGACY_ENVIRONMENT_PROPERTY), + System.getenv(LEGACY_ENVIRONMENT_VARIABLE), + System.getProperty(WAR_CONTEXT_PROPERTY) + ); + if (configuredName != null) { + return configuredName; + } + if (ctx == null) { + return ""; + } + return firstNonBlank(ctx.contextPath(), ctx.req != null ? ctx.req.getContextPath() : null, ""); + } + + static boolean environmentLooksLikeDev(String environmentName) { + return normalizeEnvironmentName(environmentName).contains(DEV_MARKER); + } + + static String normalizeEnvironmentName(String environmentName) { + if (environmentName == null) { + return ""; + } + return environmentName.toLowerCase().replaceAll("[^a-z0-9]", ""); + } + + private static String stackTraceOf(Throwable cause) { + StringWriter sw = new StringWriter(); + cause.printStackTrace(new PrintWriter(sw)); + return sw.toString(); + } + + private static String firstNonBlank(String... values) { + for (String value : values) { + if (value != null && !value.isBlank()) { + return value; + } + } + return null; + } +} diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java new file mode 100644 index 0000000000..731999a0d5 --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java @@ -0,0 +1,124 @@ +package cwms.cda.api.errors; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import cwms.cda.security.DataApiPrincipal; +import cwms.cda.security.Role; +import java.io.Serializable; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +class ErrorTraceSupportTest { + + @AfterEach + void clearProperties() { + System.clearProperty(ErrorTraceSupport.ALWAYS_SHOW_STACK_TRACE_PROPERTY); + System.clearProperty(ErrorTraceSupport.PRIMARY_ENVIRONMENT_PROPERTY); + System.clearProperty(ErrorTraceSupport.LEGACY_ENVIRONMENT_PROPERTY); + System.clearProperty(ErrorTraceSupport.WAR_CONTEXT_PROPERTY); + } + + @Test + void includesStackTraceForDevAdminUser() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + Map details = ErrorTraceSupport.buildDetails(Map.of(), + new IllegalStateException("boom"), + ErrorTraceSupport.shouldIncludeStackTrace(principal, "CWMS_DEV-West")); + + assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + assertTrue(details.get(ErrorTraceSupport.STACK_TRACE_KEY).toString() + .contains("IllegalStateException")); + } + + @Test + void includesStackTraceForExistingUserAdminsRole() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + assertTrue(ErrorTraceSupport.shouldIncludeStackTrace(principal, "spk-dev")); + } + + @Test + void omitsStackTraceOutsideDev() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + Map details = ErrorTraceSupport.buildDetails(Map.of(), + new IllegalStateException("boom"), + ErrorTraceSupport.shouldIncludeStackTrace(principal, "production")); + + assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + } + + @Test + void omitsStackTraceForNonAdminUser() { + DataApiPrincipal principal = new DataApiPrincipal("normal-user", + Set.of(new Role("CWMS Users"))); + + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, "dev")); + } + + @Test + void omitsStackTraceForGuestUser() { + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(null, "dev")); + } + + @Test + void normalizesEnvironmentNameBeforeDevCheck() { + assertTrue(ErrorTraceSupport.environmentLooksLikeDev("CWMS.Dev-West")); + assertTrue(ErrorTraceSupport.environmentLooksLikeDev("cwms_dev_west")); + assertFalse(ErrorTraceSupport.environmentLooksLikeDev("production")); + } + + @Test + void fallsBackToContextPathWhenEnvironmentPropertyMissing() { + assertTrue(ErrorTraceSupport.environmentLooksLikeDev("/spk-data-dev")); + } + + @Test + void environmentPropertyStillResolvesForDevChecks() { + System.setProperty(ErrorTraceSupport.PRIMARY_ENVIRONMENT_PROPERTY, "local-dev"); + + assertTrue(ErrorTraceSupport.environmentLooksLikeDev( + ErrorTraceSupport.resolveEnvironmentName(null))); + } + + @Test + void omitsStackTraceForUndefinedCwmsAdminRole() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS Admin"))); + + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, "dev")); + } + + @Test + void overrideEnablesStackTraceWithoutAdminRole() { + DataApiPrincipal principal = new DataApiPrincipal("normal-user", + Set.of(new Role("CWMS Users"))); + System.setProperty(ErrorTraceSupport.ALWAYS_SHOW_STACK_TRACE_PROPERTY, "true"); + + assertTrue(ErrorTraceSupport.shouldIncludeStackTrace(principal, "production")); + } + + @Test + void overrideAddsStackTraceToDetails() { + System.setProperty(ErrorTraceSupport.ALWAYS_SHOW_STACK_TRACE_PROPERTY, "true"); + + Map details = ErrorTraceSupport.buildDetails(Map.of(), + new IllegalStateException("boom"), + ErrorTraceSupport.shouldIncludeStackTrace(null, "production")); + + assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + } + + @Test + void localhostRequestEnablesStackTraceWithoutAuth() { + assertTrue(ErrorTraceSupport.localhostRequestOverrideEnabled("localhost")); + assertTrue(ErrorTraceSupport.localhostRequestOverrideEnabled("127.0.0.1")); + assertFalse(ErrorTraceSupport.localhostRequestOverrideEnabled("example.com")); + } +} From 982fa8a35c925f01ad9bb67881012a3085ffda25 Mon Sep 17 00:00:00 2001 From: Charles Graham Date: Wed, 8 Apr 2026 21:46:36 -0500 Subject: [PATCH 02/10] Cleanup endpoint specific errors to use ErrorTraceSupport, preserving behavior and keeping it consistent --- .../java/cwms/cda/api/BasinController.java | 3 ++- .../cda/api/BinaryTimeSeriesController.java | 5 +++-- .../java/cwms/cda/api/LocationController.java | 13 +++++------- .../cda/api/TextTimeSeriesController.java | 5 +++-- .../cwms/cda/api/TimeSeriesController.java | 5 +++-- ...eSeriesIdentifierDescriptorController.java | 3 ++- .../cwms/cda/api/rating/RatingController.java | 21 ++++++++++++------- 7 files changed, 31 insertions(+), 24 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java b/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java index c8bdf4f4a6..d66174f981 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java @@ -42,6 +42,7 @@ import com.codahale.metrics.Timer; import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.basinconnectivity.BasinDao; import cwms.cda.data.dto.CwmsId; @@ -143,7 +144,7 @@ public void getAll(@NotNull Context ctx) { ctx.result(result); ctx.status(HttpServletResponse.SC_OK); } catch (SQLException ex) { - CdaError error = new CdaError("Error retrieving all basins"); + CdaError error = ErrorTraceSupport.buildError(ctx, "Error retrieving all basins", ex); LOGGER.atSevere().withCause(ex).log("Error retrieving all basins"); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(error); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java index 8f6f1d0e1e..980151ac91 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java @@ -45,6 +45,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.binarytimeseries.TimeSeriesBinaryDao; import cwms.cda.data.dto.binarytimeseries.BinaryTimeSeries; import cwms.cda.formatters.ContentType; @@ -163,8 +164,8 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); } catch (URISyntaxException | UnsupportedEncodingException ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java b/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java index f1610e113d..13188ea6f2 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java @@ -43,6 +43,7 @@ import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; import cwms.cda.api.errors.DeleteConflictException; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.LocationsDao; import cwms.cda.data.dao.LocationsDaoImpl; import cwms.cda.data.dto.Location; @@ -200,10 +201,6 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = new CdaError("failed to process request"); - logger.atSevere().withCause(ex).log("%s", re.toString()); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } @@ -261,7 +258,7 @@ public void getOne(@NotNull Context ctx, @NotNull String locationId) { addDeprecatedContentTypeWarning(ctx, contentType); } catch (IOException ex) { String errorMsg = "Error retrieving " + locationId; - CdaError re = new CdaError(errorMsg); + CdaError re = ErrorTraceSupport.buildError(ctx, errorMsg, ex); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); logger.atSevere().withCause(ex).log("%s", errorMsg); } @@ -303,7 +300,7 @@ public void create(@NotNull Context ctx) { "Created Location", locationFromBody.getName()); ctx.status(HttpServletResponse.SC_CREATED).json(re); } catch (IOException ex) { - CdaError re = new CdaError("failed to process request"); + CdaError re = ErrorTraceSupport.buildError(ctx, "failed to process request", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -357,8 +354,8 @@ public void update(@NotNull Context ctx, @NotNull String locationId) { "Updated Location", updatedLocation.getName())); } } catch (IOException ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java index 43893dd3a9..521f4e945a 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java @@ -30,6 +30,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.texttimeseries.TimeSeriesTextDao; import cwms.cda.data.dto.texttimeseries.TextTimeSeries; import cwms.cda.formatters.ContentType; @@ -144,8 +145,8 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); } catch (URISyntaxException | UnsupportedEncodingException ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java index 6a7841ff27..580e82cbaf 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java @@ -47,6 +47,7 @@ import com.codahale.metrics.Timer; import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.api.errors.NotFoundException; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.StoreRule; @@ -222,7 +223,7 @@ public void create(@NotNull Context ctx) { dao.create(timeSeries, createAsLrts, storeRule, overrideProtection, vd); ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException | IOException ex) { - CdaError re = new CdaError("Internal Error"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -638,7 +639,7 @@ public void update(@NotNull Context ctx, @NotNull String id) { ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException | IOException ex) { - CdaError re = new CdaError("Internal Error"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java index 142a430c66..f9958a4315 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java @@ -31,6 +31,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.TimeSeriesIdentifierDescriptorDao; import cwms.cda.data.dto.TimeSeriesIdentifierDescriptor; @@ -343,7 +344,7 @@ public void delete(@NotNull Context ctx, @NotNull String timeseriesId) { ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException ex) { - CdaError re = new CdaError("Internal Error"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java index 3658162424..763acee65c 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java @@ -62,6 +62,7 @@ import com.fasterxml.jackson.dataformat.xml.XmlMapper; import cwms.cda.api.Controllers; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.JsonRatingUtils; import cwms.cda.data.dao.RatingDao; import cwms.cda.data.dao.RatingSetDao; @@ -171,11 +172,13 @@ public void create(@NotNull Context ctx) { StatusResponse re = new StatusResponse(RatingDao.extractOfficeFromXml(ratingSet), "Rating Set successfully stored to CWMS."); ctx.status(HttpServletResponse.SC_CREATED).json(re); } catch (IOException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet", ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } catch (RatingException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -492,14 +495,14 @@ private String getRatingSetString(Context ctx, RatingSet.DatabaseLoadMethod meth ctx.status(HttpCode.NOT_FOUND); } } catch (RatingException e) { - CdaError re = - new CdaError("Failed to process request to retrieve RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to retrieve RatingSet", e); logger.atSevere().withCause(e).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); ctx.json(re); } catch (IOException e) { - CdaError re = - new CdaError("Failed to process request to retrieve RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to retrieve RatingSet", e); logger.atSevere().withCause(e).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -580,11 +583,13 @@ public void update(@NotNull Context ctx, @NotNull String ratingId) { StatusResponse re = new StatusResponse(RatingDao.extractOfficeFromXml(ratingSet), "Updated RatingSet"); ctx.status(HttpServletResponse.SC_OK).json(re); } catch (IOException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet", ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } catch (RatingException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } From a2f50dc880422bd10097bd997d1c3d33da6ab70b Mon Sep 17 00:00:00 2001 From: Charles Graham Date: Wed, 8 Apr 2026 21:47:11 -0500 Subject: [PATCH 03/10] Allow generic 500 errors to bubble up to ApiServlet --- .../src/main/java/cwms/cda/api/ParametersController.java | 6 ------ .../main/java/cwms/cda/api/SpecifiedLevelController.java | 7 ------- .../src/main/java/cwms/cda/api/TimeZoneController.java | 7 ------- .../src/main/java/cwms/cda/api/UnitsController.java | 6 ------ .../java/cwms/cda/api/rating/RatingMetadataController.java | 5 ----- .../java/cwms/cda/api/rating/RatingSpecController.java | 5 ----- .../java/cwms/cda/api/rating/RatingTemplateController.java | 5 ----- 7 files changed, 41 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java b/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java index b3a103d451..b4fa06aae2 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java @@ -28,12 +28,10 @@ import io.javalin.plugin.openapi.annotations.OpenApiParam; import io.javalin.plugin.openapi.annotations.OpenApiResponse; import java.util.List; -import com.google.common.flogger.FluentLogger; import javax.servlet.http.HttpServletResponse; import org.jooq.DSLContext; public class ParametersController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final MetricRegistry metrics; private final Histogram requestResultSize; @@ -121,10 +119,6 @@ public void getAll(Context ctx) { ctx.result(results); addDeprecatedContentTypeWarning(ctx, contentType); requestResultSize.update(results.length()); - } catch (Exception ex) { - CdaError re = new CdaError("Failed to process request"); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java b/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java index 31638a5d43..db2ac2b9f6 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java @@ -41,7 +41,6 @@ import javax.servlet.http.HttpServletResponse; import java.util.List; -import com.google.common.flogger.FluentLogger; import static com.codahale.metrics.MetricRegistry.name; import static cwms.cda.api.Controllers.*; @@ -49,7 +48,6 @@ public class SpecifiedLevelController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String TAG = "Levels"; private final MetricRegistry metrics; @@ -110,11 +108,6 @@ public void getAll(Context ctx) { ctx.result(result); requestResultSize.update(result.length()); ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java index 44cf6d9ff3..fa1ca5c6f2 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java @@ -30,13 +30,10 @@ import io.javalin.plugin.openapi.annotations.OpenApiParam; import io.javalin.plugin.openapi.annotations.OpenApiResponse; -import com.google.common.flogger.FluentLogger; import javax.servlet.http.HttpServletResponse; import org.jooq.DSLContext; public class TimeZoneController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final MetricRegistry metrics; private final Histogram requestResultSize; @@ -121,10 +118,6 @@ public void getAll(Context ctx) { requestResultSize.update(results.length()); ctx.status(HttpServletResponse.SC_OK); ctx.result(results); - } catch (Exception ex) { - logger.atSevere().withCause(ex).log("Failed to process request"); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - ctx.result("Failed to process request"); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java b/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java index 11f1b2aa86..89fb497ead 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java @@ -27,12 +27,10 @@ import io.javalin.plugin.openapi.annotations.OpenApiParam; import io.javalin.plugin.openapi.annotations.OpenApiResponse; import java.util.List; -import com.google.common.flogger.FluentLogger; import javax.servlet.http.HttpServletResponse; import org.jooq.DSLContext; public class UnitsController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final MetricRegistry metrics; private final Histogram requestResultSize; @@ -115,10 +113,6 @@ public void getAll(Context ctx) { ctx.result(results); addDeprecatedContentTypeWarning(ctx, contentType); requestResultSize.update(results.length()); - } catch (Exception ex) { - logger.atSevere().withCause(ex).log("Failed to process request"); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - ctx.result("Failed to process request"); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java index ce95a2f178..bd684d0e91 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java @@ -167,11 +167,6 @@ public void getAll(Context ctx) { requestResultSize.update(result.length()); ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java index 63f157b450..41eb577b58 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java @@ -138,11 +138,6 @@ public void getAll(Context ctx) { ctx.result(result); requestResultSize.update(result.length()); ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java index a56b6ed50c..6f55c1039b 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java @@ -134,11 +134,6 @@ public void getAll(Context ctx) { String result = Formats.format(contentType, ratingTemplates); ctx.result(result); requestResultSize.update(result.length()); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } From cfbc4d09c0ab6cdff0b0e77ec9b9ef7c2cbb4ce3 Mon Sep 17 00:00:00 2001 From: Charles Graham Date: Wed, 8 Apr 2026 22:16:03 -0500 Subject: [PATCH 04/10] Update compose readme to include the changes --- docker-compose.README.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docker-compose.README.md b/docker-compose.README.md index 5a0be00da2..006bd1b7d9 100644 --- a/docker-compose.README.md +++ b/docker-compose.README.md @@ -45,3 +45,26 @@ The following users and permissions are available: Traefik uses port 8081 by default, if this conflicts with existing services on your machine it can be changed by setting the APP_PORT variable. + +## Local error trace debugging + +The API supports conditional stack traces in JSON error responses through +`cwms.cda.api.errors.ErrorTraceSupport`. + +For local Docker Compose or localhost testing, stack traces can be enabled in either of these ways: + +1. Set the explicit override: + + `CWMS_DATAAPI_ERRORS_ALWAYS_SHOW_STACK_TRACE=true` + +2. Run with an environment name that normalizes to a value containing `dev`, for example: + + `ENVIRONMENT=development` + +In addition, localhost requests are treated as local debug requests. When enabled, error responses +include `details.stackTrace` along with the existing `incidentIdentifier`. + +This is intended for local development and shared `development` environments. In shared `dev` (not localhost), +stack traces are gated by authentication and the `CWMS User Admins` role. + +Response stack traces should *not* be enabled broadly for production traffic. From bffb28683521ba2df41746e39687696c8edaa4f9 Mon Sep 17 00:00:00 2001 From: Charles Graham Date: Wed, 8 Apr 2026 22:43:38 -0500 Subject: [PATCH 05/10] Add list of stack traces split by new line for visiblity in browser --- .../java/cwms/cda/api/errors/ErrorTraceSupport.java | 13 ++++++++++++- .../cwms/cda/api/errors/ErrorTraceSupportTest.java | 6 ++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java index fbc7525e47..5fa83d8714 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java @@ -8,13 +8,16 @@ import java.io.PrintWriter; import java.io.Serializable; import java.io.StringWriter; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; public final class ErrorTraceSupport { public static final String STACK_TRACE_KEY = "stackTrace"; + public static final String STACK_TRACE_LINES_KEY = "stackTraceLines"; static final String ALWAYS_SHOW_STACK_TRACE_PROPERTY = "cwms.dataapi.errors.alwaysShowStackTrace"; static final String ALWAYS_SHOW_STACK_TRACE_VARIABLE = "CWMS_DATAAPI_ERRORS_ALWAYS_SHOW_STACK_TRACE"; static final String PRIMARY_ENVIRONMENT_PROPERTY = "cwms.dataapi.environment.name"; @@ -54,7 +57,9 @@ static Map buildDetails(Map details, merged.putAll(details); } if (cause != null && includeStackTrace) { - merged.put(STACK_TRACE_KEY, stackTraceOf(cause)); + String stackTrace = stackTraceOf(cause); + merged.put(STACK_TRACE_KEY, stackTrace); + merged.put(STACK_TRACE_LINES_KEY, stackTraceLinesOf(stackTrace)); } return Collections.unmodifiableMap(merged); } @@ -150,6 +155,12 @@ private static String stackTraceOf(Throwable cause) { return sw.toString(); } + private static ArrayList stackTraceLinesOf(String stackTrace) { + ArrayList lines = new ArrayList<>(); + Collections.addAll(lines, stackTrace.split("\\R")); + return lines; + } + private static String firstNonBlank(String... values) { for (String value : values) { if (value != null && !value.isBlank()) { diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java index 731999a0d5..66f8d94327 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java @@ -1,6 +1,7 @@ package cwms.cda.api.errors; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTrue; import cwms.cda.security.DataApiPrincipal; import cwms.cda.security.Role; @@ -30,8 +31,11 @@ void includesStackTraceForDevAdminUser() { ErrorTraceSupport.shouldIncludeStackTrace(principal, "CWMS_DEV-West")); assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); assertTrue(details.get(ErrorTraceSupport.STACK_TRACE_KEY).toString() .contains("IllegalStateException")); + assertTrue(assertInstanceOf(Iterable.class, details.get(ErrorTraceSupport.STACK_TRACE_LINES_KEY)) + .iterator().next().toString().contains("IllegalStateException")); } @Test @@ -52,6 +56,7 @@ void omitsStackTraceOutsideDev() { ErrorTraceSupport.shouldIncludeStackTrace(principal, "production")); assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); } @Test @@ -113,6 +118,7 @@ void overrideAddsStackTraceToDetails() { ErrorTraceSupport.shouldIncludeStackTrace(null, "production")); assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); } @Test From 9bcae0408de0c7f90b366cb55e21f1343cb7991a Mon Sep 17 00:00:00 2001 From: Charles Graham Date: Thu, 9 Apr 2026 18:50:06 -0500 Subject: [PATCH 06/10] check togglz flag instead of env/property, default it off --- .../cda/api/errors/ErrorTraceSupport.java | 72 ++++--------------- .../java/cwms/cda/features/CdaFeatures.java | 4 +- .../src/main/resources/features.properties | 3 +- .../cda/api/errors/ErrorTraceSupportTest.java | 68 +++--------------- .../CdaFeatureManagerProviderTest.java | 18 +++++ docker-compose.README.md | 20 ++---- 6 files changed, 51 insertions(+), 134 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java index 5fa83d8714..98f2d0cc19 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java @@ -1,6 +1,7 @@ package cwms.cda.api.errors; import cwms.cda.data.dao.AuthDao; +import cwms.cda.features.CdaFeatures; import cwms.cda.security.DataApiPrincipal; import cwms.cda.security.Role; import cwms.cda.spi.IdentityProvider; @@ -11,21 +12,14 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Optional; +import org.togglz.core.context.FeatureContext; +import org.togglz.core.manager.FeatureManager; public final class ErrorTraceSupport { public static final String STACK_TRACE_KEY = "stackTrace"; public static final String STACK_TRACE_LINES_KEY = "stackTraceLines"; - static final String ALWAYS_SHOW_STACK_TRACE_PROPERTY = "cwms.dataapi.errors.alwaysShowStackTrace"; - static final String ALWAYS_SHOW_STACK_TRACE_VARIABLE = "CWMS_DATAAPI_ERRORS_ALWAYS_SHOW_STACK_TRACE"; - static final String PRIMARY_ENVIRONMENT_PROPERTY = "cwms.dataapi.environment.name"; - static final String PRIMARY_ENVIRONMENT_VARIABLE = "CWMS_DATAAPI_ENVIRONMENT_NAME"; - static final String HOST_ENVIRONMENT_VARIABLE = "ENVIRONMENT"; - static final String LEGACY_ENVIRONMENT_PROPERTY = "cda.environment.name"; - static final String LEGACY_ENVIRONMENT_VARIABLE = "CDA_ENVIRONMENT_NAME"; - static final String WAR_CONTEXT_PROPERTY = "warContext"; private static final String DEV_MARKER = "dev"; private static final Role CWMS_USER_ADMINS_ROLE = new Role("CWMS User Admins"); @@ -65,28 +59,14 @@ static Map buildDetails(Map details, } static boolean shouldIncludeStackTrace(Context ctx) { - if (alwaysShowStackTraceOverrideEnabled()) { - return true; - } if (localhostRequestOverrideEnabled(ctx)) { return true; } - return shouldIncludeStackTrace(resolvePrincipal(ctx).orElse(null), resolveEnvironmentName(ctx)); - } - - static boolean shouldIncludeStackTrace(DataApiPrincipal principal, String environmentName) { - if (alwaysShowStackTraceOverrideEnabled()) { - return true; - } - return hasAdminRole(principal) && environmentLooksLikeDev(environmentName); + return shouldIncludeStackTrace(resolvePrincipal(ctx).orElse(null), stackTraceFeatureEnabled()); } - static boolean alwaysShowStackTraceOverrideEnabled() { - return Boolean.parseBoolean(firstNonBlank( - System.getProperty(ALWAYS_SHOW_STACK_TRACE_PROPERTY), - System.getenv(ALWAYS_SHOW_STACK_TRACE_VARIABLE), - "false" - )); + static boolean shouldIncludeStackTrace(DataApiPrincipal principal, boolean stackTraceFeatureEnabled) { + return stackTraceFeatureEnabled && hasAdminRole(principal); } static boolean localhostRequestOverrideEnabled(Context ctx) { @@ -120,33 +100,13 @@ static boolean hasAdminRole(DataApiPrincipal principal) { && principal.getRoles().contains(CWMS_USER_ADMINS_ROLE); } - static String resolveEnvironmentName(Context ctx) { - String configuredName = firstNonBlank( - System.getProperty(PRIMARY_ENVIRONMENT_PROPERTY), - System.getenv(PRIMARY_ENVIRONMENT_VARIABLE), - System.getenv(HOST_ENVIRONMENT_VARIABLE), - System.getProperty(LEGACY_ENVIRONMENT_PROPERTY), - System.getenv(LEGACY_ENVIRONMENT_VARIABLE), - System.getProperty(WAR_CONTEXT_PROPERTY) - ); - if (configuredName != null) { - return configuredName; - } - if (ctx == null) { - return ""; - } - return firstNonBlank(ctx.contextPath(), ctx.req != null ? ctx.req.getContextPath() : null, ""); - } - - static boolean environmentLooksLikeDev(String environmentName) { - return normalizeEnvironmentName(environmentName).contains(DEV_MARKER); - } - - static String normalizeEnvironmentName(String environmentName) { - if (environmentName == null) { - return ""; + static boolean stackTraceFeatureEnabled() { + try { + FeatureManager featureManager = FeatureContext.getFeatureManager(); + return featureManager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); + } catch (Throwable ignore) { + return false; } - return environmentName.toLowerCase().replaceAll("[^a-z0-9]", ""); } private static String stackTraceOf(Throwable cause) { @@ -161,12 +121,4 @@ private static ArrayList stackTraceLinesOf(String stackTrace) { return lines; } - private static String firstNonBlank(String... values) { - for (String value : values) { - if (value != null && !value.isBlank()) { - return value; - } - } - return null; - } } diff --git a/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java b/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java index fa2a662efa..c8bcbd3dde 100644 --- a/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java +++ b/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java @@ -5,5 +5,7 @@ public enum CdaFeatures implements Feature { @Label("Use object-storage backed Blob DAO in BlobController") - USE_OBJECT_STORAGE_BLOBS + USE_OBJECT_STORAGE_BLOBS, + @Label("Include stack traces in JSON error responses for authorized debug requests") + INCLUDE_ERROR_STACK_TRACES } diff --git a/cwms-data-api/src/main/resources/features.properties b/cwms-data-api/src/main/resources/features.properties index 94eeb15567..c7c091f5d8 100644 --- a/cwms-data-api/src/main/resources/features.properties +++ b/cwms-data-api/src/main/resources/features.properties @@ -1 +1,2 @@ -USE_OBJECT_STORAGE_BLOBS=false \ No newline at end of file +USE_OBJECT_STORAGE_BLOBS=false +INCLUDE_ERROR_STACK_TRACES=false diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java index 66f8d94327..dce8e43522 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java @@ -8,27 +8,18 @@ import java.io.Serializable; import java.util.Map; import java.util.Set; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; class ErrorTraceSupportTest { - @AfterEach - void clearProperties() { - System.clearProperty(ErrorTraceSupport.ALWAYS_SHOW_STACK_TRACE_PROPERTY); - System.clearProperty(ErrorTraceSupport.PRIMARY_ENVIRONMENT_PROPERTY); - System.clearProperty(ErrorTraceSupport.LEGACY_ENVIRONMENT_PROPERTY); - System.clearProperty(ErrorTraceSupport.WAR_CONTEXT_PROPERTY); - } - @Test - void includesStackTraceForDevAdminUser() { + void includesStackTraceWhenFeatureEnabledForAdminUser() { DataApiPrincipal principal = new DataApiPrincipal("admin-user", Set.of(new Role("CWMS User Admins"))); Map details = ErrorTraceSupport.buildDetails(Map.of(), new IllegalStateException("boom"), - ErrorTraceSupport.shouldIncludeStackTrace(principal, "CWMS_DEV-West")); + ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); @@ -39,21 +30,21 @@ void includesStackTraceForDevAdminUser() { } @Test - void includesStackTraceForExistingUserAdminsRole() { + void featureEnablesStackTraceForUserAdminsRole() { DataApiPrincipal principal = new DataApiPrincipal("admin-user", Set.of(new Role("CWMS User Admins"))); - assertTrue(ErrorTraceSupport.shouldIncludeStackTrace(principal, "spk-dev")); + assertTrue(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); } @Test - void omitsStackTraceOutsideDev() { + void omitsStackTraceWhenFeatureDisabled() { DataApiPrincipal principal = new DataApiPrincipal("admin-user", Set.of(new Role("CWMS User Admins"))); Map details = ErrorTraceSupport.buildDetails(Map.of(), new IllegalStateException("boom"), - ErrorTraceSupport.shouldIncludeStackTrace(principal, "production")); + ErrorTraceSupport.shouldIncludeStackTrace(principal, false)); assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); @@ -64,32 +55,12 @@ void omitsStackTraceForNonAdminUser() { DataApiPrincipal principal = new DataApiPrincipal("normal-user", Set.of(new Role("CWMS Users"))); - assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, "dev")); + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); } @Test void omitsStackTraceForGuestUser() { - assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(null, "dev")); - } - - @Test - void normalizesEnvironmentNameBeforeDevCheck() { - assertTrue(ErrorTraceSupport.environmentLooksLikeDev("CWMS.Dev-West")); - assertTrue(ErrorTraceSupport.environmentLooksLikeDev("cwms_dev_west")); - assertFalse(ErrorTraceSupport.environmentLooksLikeDev("production")); - } - - @Test - void fallsBackToContextPathWhenEnvironmentPropertyMissing() { - assertTrue(ErrorTraceSupport.environmentLooksLikeDev("/spk-data-dev")); - } - - @Test - void environmentPropertyStillResolvesForDevChecks() { - System.setProperty(ErrorTraceSupport.PRIMARY_ENVIRONMENT_PROPERTY, "local-dev"); - - assertTrue(ErrorTraceSupport.environmentLooksLikeDev( - ErrorTraceSupport.resolveEnvironmentName(null))); + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(null, true)); } @Test @@ -97,28 +68,7 @@ void omitsStackTraceForUndefinedCwmsAdminRole() { DataApiPrincipal principal = new DataApiPrincipal("admin-user", Set.of(new Role("CWMS Admin"))); - assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, "dev")); - } - - @Test - void overrideEnablesStackTraceWithoutAdminRole() { - DataApiPrincipal principal = new DataApiPrincipal("normal-user", - Set.of(new Role("CWMS Users"))); - System.setProperty(ErrorTraceSupport.ALWAYS_SHOW_STACK_TRACE_PROPERTY, "true"); - - assertTrue(ErrorTraceSupport.shouldIncludeStackTrace(principal, "production")); - } - - @Test - void overrideAddsStackTraceToDetails() { - System.setProperty(ErrorTraceSupport.ALWAYS_SHOW_STACK_TRACE_PROPERTY, "true"); - - Map details = ErrorTraceSupport.buildDetails(Map.of(), - new IllegalStateException("boom"), - ErrorTraceSupport.shouldIncludeStackTrace(null, "production")); - - assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); - assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); } @Test diff --git a/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java b/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java index 7a2f68a2a1..8506eec9e3 100644 --- a/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java +++ b/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java @@ -61,6 +61,23 @@ void testUseObjectStorageBlobsFeature() throws IOException { assertTrue(manager.isActive(CdaFeatures.USE_OBJECT_STORAGE_BLOBS)); } + @Test + void testIncludeErrorStackTracesFeature() throws IOException { + File tempFile = Files.createTempFile("features", ".properties").toFile(); + tempFile.deleteOnExit(); + + try (FileWriter writer = new FileWriter(tempFile)) { + writer.write(CdaFeatures.INCLUDE_ERROR_STACK_TRACES.name() + " = true"); + } + + System.setProperty(CdaFeatureManagerProvider.PROPERTIES_FILE, tempFile.getAbsolutePath()); + + CdaFeatureManagerProvider provider = new CdaFeatureManagerProvider(); + FeatureManager manager = provider.getFeatureManager(); + + assertTrue(manager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES)); + } + @Test void testFeatureDisabledByDefault() throws IOException { File tempFile = Files.createTempFile("features_disabled", ".properties").toFile(); @@ -73,5 +90,6 @@ void testFeatureDisabledByDefault() throws IOException { FeatureManager manager = provider.getFeatureManager(); assertFalse(manager.isActive(CdaFeatures.USE_OBJECT_STORAGE_BLOBS)); + assertFalse(manager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES)); } } diff --git a/docker-compose.README.md b/docker-compose.README.md index 006bd1b7d9..5dde9ba3ab 100644 --- a/docker-compose.README.md +++ b/docker-compose.README.md @@ -51,20 +51,14 @@ be changed by setting the APP_PORT variable. The API supports conditional stack traces in JSON error responses through `cwms.cda.api.errors.ErrorTraceSupport`. -For local Docker Compose or localhost testing, stack traces can be enabled in either of these ways: +Stack-trace exposure is controlled through the Togglz feature flag +`INCLUDE_ERROR_STACK_TRACES`. -1. Set the explicit override: +For shared environments, enable that feature in the active `features.properties` and the API will +include traces only for authenticated users with the `CWMS User Admins` role. - `CWMS_DATAAPI_ERRORS_ALWAYS_SHOW_STACK_TRACE=true` - -2. Run with an environment name that normalizes to a value containing `dev`, for example: - - `ENVIRONMENT=development` - -In addition, localhost requests are treated as local debug requests. When enabled, error responses -include `details.stackTrace` along with the existing `incidentIdentifier`. - -This is intended for local development and shared `development` environments. In shared `dev` (not localhost), -stack traces are gated by authentication and the `CWMS User Admins` role. +For local Docker Compose or other localhost testing, requests to `localhost`, `127.0.0.1`, and +`::1` are still treated as local debug requests even when the feature is disabled. When enabled, +error responses include `details.stackTrace` along with the existing `incidentIdentifier`. Response stack traces should *not* be enabled broadly for production traffic. From 091f3db613f2d36887e037f36deeedd1b38f0e5c Mon Sep 17 00:00:00 2001 From: "Charles Graham, SWT" Date: Tue, 21 Apr 2026 17:25:46 -0500 Subject: [PATCH 07/10] Refine exception trace gating --- compose_files/sql/users.sql | 2 + .../src/main/java/cwms/cda/ApiServlet.java | 16 ++-- .../java/cwms/cda/api/BasinController.java | 4 +- .../cda/api/BinaryTimeSeriesController.java | 4 +- .../java/cwms/cda/api/LocationController.java | 8 +- .../cda/api/TextTimeSeriesController.java | 4 +- .../cwms/cda/api/TimeSeriesController.java | 6 +- ...eSeriesIdentifierDescriptorController.java | 4 +- ...upport.java => ExceptionTraceSupport.java} | 49 +++--------- .../cwms/cda/api/rating/RatingController.java | 26 +++--- .../cda/api/errors/ErrorTraceSupportTest.java | 80 ------------------- .../api/errors/ExceptionTraceSupportTest.java | 70 ++++++++++++++++ .../src/test/java/fixtures/TestAccounts.java | 2 +- .../resources/cwms/cda/data/sql/users.sql | 2 + 14 files changed, 124 insertions(+), 153 deletions(-) rename cwms-data-api/src/main/java/cwms/cda/api/errors/{ErrorTraceSupport.java => ExceptionTraceSupport.java} (66%) delete mode 100644 cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java create mode 100644 cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceSupportTest.java diff --git a/compose_files/sql/users.sql b/compose_files/sql/users.sql index e9d8329a92..c82ecbce35 100644 --- a/compose_files/sql/users.sql +++ b/compose_files/sql/users.sql @@ -10,6 +10,7 @@ begin cwms_sec.update_edipi('l2hectest', 1234567890); cwms_sec.add_user_to_group('l2hectest', 'All Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest', 'CWMS Users', 'SPK'); + cwms_sec.add_user_to_group('l2hectest', 'SHOW STACK TRACE', 'SPK'); cwms_sec.add_user_to_group('l2hectest', 'TS ID Creator', 'SPK'); cwms_sec.add_cwms_user('l1hectest', null, 'SPL'); -- intentionally no extra permissions. @@ -59,6 +60,7 @@ begin cwms_sec.add_user_to_group('m5testadmin','All Users', 'LRL'); cwms_sec.add_user_to_group('m5testadmin','CWMS Users', 'LRL'); cwms_sec.add_user_to_group('m5testadmin','CWMS User Admins', 'LRL'); + cwms_sec.add_user_to_group('m5testadmin','SHOW STACK TRACE', 'LRL'); end; / 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 8d26760a65..9b6fc4e81c 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -106,7 +106,7 @@ import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.ApplicationException; import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.api.location.kind.GateChangeCreateController; import cwms.cda.api.location.kind.GateChangeDeleteController; import cwms.cda.api.location.kind.GateChangeGetAllController; @@ -356,7 +356,7 @@ public void init() { ctx.header("X-XSS-Protection", "1; mode=block"); }) .exception(ApplicationException.class, (e, ctx) -> { - CdaError re = ErrorTraceSupport.buildError(ctx, e.getCdaErrorMessage(), + CdaError re = ExceptionTraceSupport.buildError(ctx, e.getCdaErrorMessage(), e.getSource(), e.getDetails(), e); if (e.getLoggerLevel().isPresent()) { logger.at(e.getLoggerLevel().get()).withCause(e).log(re.toString()); @@ -364,24 +364,24 @@ public void init() { ctx.status(e.getCdaHttpErrorCode()).json(re); }) .exception(UnsupportedOperationException.class, (e, ctx) -> { - final CdaError re = ErrorTraceSupport.buildError(ctx, "Not Implemented", e); + final CdaError re = ExceptionTraceSupport.buildError(ctx, "Not Implemented", e); logger.atWarning().withCause(e) .log("%s for request: %s", re, ctx.fullUrl()); ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(re); }) .exception(BadRequestResponse.class, (e, ctx) -> { - CdaError re = ErrorTraceSupport.buildError(ctx, "Bad Request", + CdaError re = ExceptionTraceSupport.buildError(ctx, "Bad Request", "User Input", new HashMap<>(e.getDetails()), e); logger.atInfo().withCause(e).log(re.toString()); ctx.status(e.getStatus()).json(re); }) .exception(IllegalArgumentException.class, (e, ctx) -> { - CdaError re = ErrorTraceSupport.buildError(ctx, "Bad Request", e); + CdaError re = ExceptionTraceSupport.buildError(ctx, "Bad Request", e); logger.atInfo().withCause(e).log(re.toString()); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DateTimeException.class, (e, ctx) -> { - CdaError re = ErrorTraceSupport.buildError(ctx, e.getMessage(), e); + CdaError re = ExceptionTraceSupport.buildError(ctx, e.getMessage(), e); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DataAccessException.class, (e, ctx) -> { @@ -395,7 +395,7 @@ public void init() { // 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 = ErrorTraceSupport.buildError(ctx, "Database Error", e); + CdaError errResponse = ExceptionTraceSupport.buildError(ctx, "Database Error", e); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); @@ -403,7 +403,7 @@ public void init() { ctx.json(errResponse); }) .exception(Exception.class, (e, ctx) -> { - CdaError errResponse = ErrorTraceSupport.buildError(ctx, "System Error", e); + CdaError errResponse = ExceptionTraceSupport.buildError(ctx, "System Error", e); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java b/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java index d66174f981..bb36013228 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java @@ -42,7 +42,7 @@ import com.codahale.metrics.Timer; import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.basinconnectivity.BasinDao; import cwms.cda.data.dto.CwmsId; @@ -144,7 +144,7 @@ public void getAll(@NotNull Context ctx) { ctx.result(result); ctx.status(HttpServletResponse.SC_OK); } catch (SQLException ex) { - CdaError error = ErrorTraceSupport.buildError(ctx, "Error retrieving all basins", ex); + CdaError error = ExceptionTraceSupport.buildError(ctx, "Error retrieving all basins", ex); LOGGER.atSevere().withCause(ex).log("Error retrieving all basins"); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(error); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java index 980151ac91..7e866b7aaa 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java @@ -45,7 +45,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.data.dao.binarytimeseries.TimeSeriesBinaryDao; import cwms.cda.data.dto.binarytimeseries.BinaryTimeSeries; import cwms.cda.formatters.ContentType; @@ -164,7 +164,7 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); } catch (URISyntaxException | UnsupportedEncodingException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, + CdaError re = ExceptionTraceSupport.buildError(ctx, "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java b/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java index 13188ea6f2..fe4c422b2d 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java @@ -43,7 +43,7 @@ import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; import cwms.cda.api.errors.DeleteConflictException; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.data.dao.LocationsDao; import cwms.cda.data.dao.LocationsDaoImpl; import cwms.cda.data.dto.Location; @@ -258,7 +258,7 @@ public void getOne(@NotNull Context ctx, @NotNull String locationId) { addDeprecatedContentTypeWarning(ctx, contentType); } catch (IOException ex) { String errorMsg = "Error retrieving " + locationId; - CdaError re = ErrorTraceSupport.buildError(ctx, errorMsg, ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, errorMsg, ex); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); logger.atSevere().withCause(ex).log("%s", errorMsg); } @@ -300,7 +300,7 @@ public void create(@NotNull Context ctx) { "Created Location", locationFromBody.getName()); ctx.status(HttpServletResponse.SC_CREATED).json(re); } catch (IOException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, "failed to process request", ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, "failed to process request", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -354,7 +354,7 @@ public void update(@NotNull Context ctx, @NotNull String locationId) { "Updated Location", updatedLocation.getName())); } } catch (IOException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, + CdaError re = ExceptionTraceSupport.buildError(ctx, "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java index 521f4e945a..993cda2a88 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java @@ -30,7 +30,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.data.dao.texttimeseries.TimeSeriesTextDao; import cwms.cda.data.dto.texttimeseries.TextTimeSeries; import cwms.cda.formatters.ContentType; @@ -145,7 +145,7 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); } catch (URISyntaxException | UnsupportedEncodingException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, + CdaError re = ExceptionTraceSupport.buildError(ctx, "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java index 580e82cbaf..659784c920 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java @@ -47,7 +47,7 @@ import com.codahale.metrics.Timer; import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.api.errors.NotFoundException; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.StoreRule; @@ -223,7 +223,7 @@ public void create(@NotNull Context ctx) { dao.create(timeSeries, createAsLrts, storeRule, overrideProtection, vd); ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException | IOException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -639,7 +639,7 @@ public void update(@NotNull Context ctx, @NotNull String id) { ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException | IOException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java index f9958a4315..3c20618afb 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java @@ -31,7 +31,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.TimeSeriesIdentifierDescriptorDao; import cwms.cda.data.dto.TimeSeriesIdentifierDescriptor; @@ -344,7 +344,7 @@ public void delete(@NotNull Context ctx, @NotNull String timeseriesId) { ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/ExceptionTraceSupport.java similarity index 66% rename from cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java rename to cwms-data-api/src/main/java/cwms/cda/api/errors/ExceptionTraceSupport.java index 98f2d0cc19..af08693af9 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/ExceptionTraceSupport.java @@ -17,13 +17,12 @@ import org.togglz.core.context.FeatureContext; import org.togglz.core.manager.FeatureManager; -public final class ErrorTraceSupport { - public static final String STACK_TRACE_KEY = "stackTrace"; +public final class ExceptionTraceSupport { public static final String STACK_TRACE_LINES_KEY = "stackTraceLines"; - private static final String DEV_MARKER = "dev"; - private static final Role CWMS_USER_ADMINS_ROLE = new Role("CWMS User Admins"); + static final String SHOW_STACK_TRACE_ROLE_NAME = "SHOW STACK TRACE"; + private static final Role SHOW_STACK_TRACE_ROLE = new Role(SHOW_STACK_TRACE_ROLE_NAME); - private ErrorTraceSupport() { + private ExceptionTraceSupport() { } public static CdaError buildError(Context ctx, String message, Throwable cause) { @@ -51,36 +50,18 @@ static Map buildDetails(Map details, merged.putAll(details); } if (cause != null && includeStackTrace) { - String stackTrace = stackTraceOf(cause); - merged.put(STACK_TRACE_KEY, stackTrace); - merged.put(STACK_TRACE_LINES_KEY, stackTraceLinesOf(stackTrace)); + merged.put(STACK_TRACE_LINES_KEY, stackTraceLinesOf(cause)); } return Collections.unmodifiableMap(merged); } static boolean shouldIncludeStackTrace(Context ctx) { - if (localhostRequestOverrideEnabled(ctx)) { - return true; - } - return shouldIncludeStackTrace(resolvePrincipal(ctx).orElse(null), stackTraceFeatureEnabled()); + return stackTraceFeatureEnabled() + && resolvePrincipal(ctx).map(ExceptionTraceSupport::hasStackTraceRole).orElse(false); } static boolean shouldIncludeStackTrace(DataApiPrincipal principal, boolean stackTraceFeatureEnabled) { - return stackTraceFeatureEnabled && hasAdminRole(principal); - } - - static boolean localhostRequestOverrideEnabled(Context ctx) { - if (ctx == null || ctx.req == null) { - return false; - } - return localhostRequestOverrideEnabled(ctx.req.getServerName()); - } - - static boolean localhostRequestOverrideEnabled(String serverName) { - return serverName != null - && ("localhost".equalsIgnoreCase(serverName) - || "127.0.0.1".equals(serverName) - || "::1".equals(serverName)); + return stackTraceFeatureEnabled && hasStackTraceRole(principal); } static Optional resolvePrincipal(Context ctx) { @@ -95,29 +76,25 @@ static Optional resolvePrincipal(Context ctx) { return Optional.ofNullable(sessionPrincipal); } - static boolean hasAdminRole(DataApiPrincipal principal) { + static boolean hasStackTraceRole(DataApiPrincipal principal) { return principal != null - && principal.getRoles().contains(CWMS_USER_ADMINS_ROLE); + && principal.getRoles().contains(SHOW_STACK_TRACE_ROLE); } static boolean stackTraceFeatureEnabled() { try { FeatureManager featureManager = FeatureContext.getFeatureManager(); return featureManager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); - } catch (Throwable ignore) { + } catch (Exception ignore) { return false; } } - private static String stackTraceOf(Throwable cause) { + private static ArrayList stackTraceLinesOf(Throwable cause) { StringWriter sw = new StringWriter(); cause.printStackTrace(new PrintWriter(sw)); - return sw.toString(); - } - - private static ArrayList stackTraceLinesOf(String stackTrace) { ArrayList lines = new ArrayList<>(); - Collections.addAll(lines, stackTrace.split("\\R")); + Collections.addAll(lines, sw.toString().split("\\R")); return lines; } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java index 763acee65c..71e4f02b57 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java @@ -62,7 +62,7 @@ import com.fasterxml.jackson.dataformat.xml.XmlMapper; import cwms.cda.api.Controllers; import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ErrorTraceSupport; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.data.dao.JsonRatingUtils; import cwms.cda.data.dao.RatingDao; import cwms.cda.data.dao.RatingSetDao; @@ -172,13 +172,13 @@ public void create(@NotNull Context ctx) { StatusResponse re = new StatusResponse(RatingDao.extractOfficeFromXml(ratingSet), "Rating Set successfully stored to CWMS."); ctx.status(HttpServletResponse.SC_CREATED).json(re); } catch (IOException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, - "Failed to process request to update RatingSet", ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet", ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } catch (RatingException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, - "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -495,14 +495,14 @@ private String getRatingSetString(Context ctx, RatingSet.DatabaseLoadMethod meth ctx.status(HttpCode.NOT_FOUND); } } catch (RatingException e) { - CdaError re = ErrorTraceSupport.buildError(ctx, - "Failed to process request to retrieve RatingSet", e); + CdaError re = ExceptionTraceSupport.buildError(ctx, + "Failed to process request to retrieve RatingSet", e); logger.atSevere().withCause(e).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); ctx.json(re); } catch (IOException e) { - CdaError re = ErrorTraceSupport.buildError(ctx, - "Failed to process request to retrieve RatingSet", e); + CdaError re = ExceptionTraceSupport.buildError(ctx, + "Failed to process request to retrieve RatingSet", e); logger.atSevere().withCause(e).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -583,13 +583,13 @@ public void update(@NotNull Context ctx, @NotNull String ratingId) { StatusResponse re = new StatusResponse(RatingDao.extractOfficeFromXml(ratingSet), "Updated RatingSet"); ctx.status(HttpServletResponse.SC_OK).json(re); } catch (IOException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, - "Failed to process request to update RatingSet", ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet", ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } catch (RatingException ex) { - CdaError re = ErrorTraceSupport.buildError(ctx, - "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); + CdaError re = ExceptionTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java deleted file mode 100644 index dce8e43522..0000000000 --- a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java +++ /dev/null @@ -1,80 +0,0 @@ -package cwms.cda.api.errors; - -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertTrue; -import cwms.cda.security.DataApiPrincipal; -import cwms.cda.security.Role; -import java.io.Serializable; -import java.util.Map; -import java.util.Set; -import org.junit.jupiter.api.Test; - -class ErrorTraceSupportTest { - - @Test - void includesStackTraceWhenFeatureEnabledForAdminUser() { - DataApiPrincipal principal = new DataApiPrincipal("admin-user", - Set.of(new Role("CWMS User Admins"))); - - Map details = ErrorTraceSupport.buildDetails(Map.of(), - new IllegalStateException("boom"), - ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); - - assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); - assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); - assertTrue(details.get(ErrorTraceSupport.STACK_TRACE_KEY).toString() - .contains("IllegalStateException")); - assertTrue(assertInstanceOf(Iterable.class, details.get(ErrorTraceSupport.STACK_TRACE_LINES_KEY)) - .iterator().next().toString().contains("IllegalStateException")); - } - - @Test - void featureEnablesStackTraceForUserAdminsRole() { - DataApiPrincipal principal = new DataApiPrincipal("admin-user", - Set.of(new Role("CWMS User Admins"))); - - assertTrue(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); - } - - @Test - void omitsStackTraceWhenFeatureDisabled() { - DataApiPrincipal principal = new DataApiPrincipal("admin-user", - Set.of(new Role("CWMS User Admins"))); - - Map details = ErrorTraceSupport.buildDetails(Map.of(), - new IllegalStateException("boom"), - ErrorTraceSupport.shouldIncludeStackTrace(principal, false)); - - assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); - assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); - } - - @Test - void omitsStackTraceForNonAdminUser() { - DataApiPrincipal principal = new DataApiPrincipal("normal-user", - Set.of(new Role("CWMS Users"))); - - assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); - } - - @Test - void omitsStackTraceForGuestUser() { - assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(null, true)); - } - - @Test - void omitsStackTraceForUndefinedCwmsAdminRole() { - DataApiPrincipal principal = new DataApiPrincipal("admin-user", - Set.of(new Role("CWMS Admin"))); - - assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); - } - - @Test - void localhostRequestEnablesStackTraceWithoutAuth() { - assertTrue(ErrorTraceSupport.localhostRequestOverrideEnabled("localhost")); - assertTrue(ErrorTraceSupport.localhostRequestOverrideEnabled("127.0.0.1")); - assertFalse(ErrorTraceSupport.localhostRequestOverrideEnabled("example.com")); - } -} diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceSupportTest.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceSupportTest.java new file mode 100644 index 0000000000..4bced27637 --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceSupportTest.java @@ -0,0 +1,70 @@ +package cwms.cda.api.errors; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertTrue; +import cwms.cda.security.DataApiPrincipal; +import cwms.cda.security.Role; +import java.io.Serializable; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; + +class ExceptionTraceSupportTest { + + @Test + void includesStackTraceLinesWhenFeatureEnabledForTraceRole() { + DataApiPrincipal principal = new DataApiPrincipal("trace-user", + Set.of(new Role(ExceptionTraceSupport.SHOW_STACK_TRACE_ROLE_NAME))); + + Map details = ExceptionTraceSupport.buildDetails(Map.of(), + new IllegalStateException("boom"), + ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + + assertFalse(details.containsKey("stackTrace")); + assertTrue(details.containsKey(ExceptionTraceSupport.STACK_TRACE_LINES_KEY)); + assertTrue(assertInstanceOf(Iterable.class, details.get(ExceptionTraceSupport.STACK_TRACE_LINES_KEY)) + .iterator().next().toString().contains("IllegalStateException")); + } + + @Test + void featureEnablesStackTraceForTraceRole() { + DataApiPrincipal principal = new DataApiPrincipal("trace-user", + Set.of(new Role(ExceptionTraceSupport.SHOW_STACK_TRACE_ROLE_NAME))); + + assertTrue(ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + } + + @Test + void omitsStackTraceWhenFeatureDisabled() { + DataApiPrincipal principal = new DataApiPrincipal("trace-user", + Set.of(new Role(ExceptionTraceSupport.SHOW_STACK_TRACE_ROLE_NAME))); + + Map details = ExceptionTraceSupport.buildDetails(Map.of(), + new IllegalStateException("boom"), + ExceptionTraceSupport.shouldIncludeStackTrace(principal, false)); + + assertFalse(details.containsKey(ExceptionTraceSupport.STACK_TRACE_LINES_KEY)); + } + + @Test + void omitsStackTraceForDifferentRole() { + DataApiPrincipal principal = new DataApiPrincipal("normal-user", + Set.of(new Role("CWMS Users"))); + + assertFalse(ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + } + + @Test + void omitsStackTraceForGuestUser() { + assertFalse(ExceptionTraceSupport.shouldIncludeStackTrace(null, true)); + } + + @Test + void omitsStackTraceForUserAdminsRoleWithoutTraceRole() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + assertFalse(ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + } +} diff --git a/cwms-data-api/src/test/java/fixtures/TestAccounts.java b/cwms-data-api/src/test/java/fixtures/TestAccounts.java index 6e81f308a1..671de481e9 100644 --- a/cwms-data-api/src/test/java/fixtures/TestAccounts.java +++ b/cwms-data-api/src/test/java/fixtures/TestAccounts.java @@ -36,7 +36,7 @@ public enum KeyUser { GUEST("guest",null,null,null, null, null), // USED as marker label for tests SPK_NORMAL("l2hectest","l2hectest","1234567890","l2userkey","ATotallyRandomStringL2hectest","SPK", "CWMS Users", ApiServlet.CAC_USER), SPK_OTHER_NORMAL_SAME_ROLES("l2hectest8","l2hectest8","12345678908","l2userkey8","ATotallyRandomStringL2hectest8","SPK", "CWMS Users", ApiServlet.CAC_USER), - SPK_NORMAL2("l2hectest_vt","l2hectestvt","2345678901","l2userkey2","DiffrntStringL2hectest_vt","SPK", "CWMS Users", ApiServlet.CAC_USER, "CWMS User Admins"), + SPK_NORMAL2("l2hectest_vt","l2hectestvt","2345678901","l2userkey2","DiffrntStringL2hectest_vt","SPK", "CWMS Users", ApiServlet.CAC_USER, "CWMS User Admins", "SHOW STACK TRACE"), SWT_NORMAL("m5hectest","swt99db","1234567890","testkey2","ATotallyRandomStringM5hectest","SWT", "CWMS Users", ApiServlet.CAC_USER), SPK_NO_ROLES("user2","user2",null,"User2key","user2SEssion", "SPK"); diff --git a/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql b/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql index 8d6136b83f..177df8df2a 100644 --- a/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql +++ b/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql @@ -82,11 +82,13 @@ begin cwms_sec.add_user_to_group('l2hectest_vt','CWMS PD Users', 'HQ'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS User Admins', 'HQ'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS DBA Users', 'HQ'); + cwms_sec.add_user_to_group('l2hectest_vt','SHOW STACK TRACE', 'HQ'); cwms_sec.add_user_to_group('l2hectest_vt','All Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS PD Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS DBA Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS User Admins', 'SPK'); + cwms_sec.add_user_to_group('l2hectest_vt','SHOW STACK TRACE', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','TS ID Creator','SPK'); cwms_sec.add_cwms_user('m5hectest', NULL, 'SWT'); From 9b21b375ffb0b65e812cfdc1b06d70491693e7e3 Mon Sep 17 00:00:00 2001 From: "Charles Graham, SWT" Date: Tue, 21 Apr 2026 17:26:14 -0500 Subject: [PATCH 08/10] Let ApiServlet handle tsid delete failures --- .../cda/api/TimeSeriesIdentifierDescriptorController.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java index 3c20618afb..63c6d2591e 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java @@ -30,8 +30,6 @@ import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; -import cwms.cda.api.errors.CdaError; -import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.TimeSeriesIdentifierDescriptorDao; import cwms.cda.data.dto.TimeSeriesIdentifierDescriptor; @@ -56,7 +54,6 @@ import javax.servlet.http.HttpServletResponse; import org.jetbrains.annotations.NotNull; import org.jooq.DSLContext; -import org.jooq.exception.DataAccessException; public class TimeSeriesIdentifierDescriptorController implements CrudHandler { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -342,11 +339,6 @@ public void delete(@NotNull Context ctx, @NotNull String timeseriesId) { dao.delete(office, timeseriesId, method); ctx.status(HttpServletResponse.SC_OK); - - } catch (DataAccessException ex) { - CdaError re = ExceptionTraceSupport.buildError(ctx, "Internal Error", ex); - logger.atSevere().withCause(ex).log("%s", re.toString()); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } From 1f0898a0aad6a8e50fbf04185113903584f3664c Mon Sep 17 00:00:00 2001 From: "Charles Graham, SWT" Date: Tue, 21 Apr 2026 17:27:07 -0500 Subject: [PATCH 09/10] Enable local compose exception traces --- compose_files/togglz/features.properties | 2 ++ docker-compose.README.md | 13 ++++++++----- docker-compose.yml | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 compose_files/togglz/features.properties diff --git a/compose_files/togglz/features.properties b/compose_files/togglz/features.properties new file mode 100644 index 0000000000..663171a401 --- /dev/null +++ b/compose_files/togglz/features.properties @@ -0,0 +1,2 @@ +USE_OBJECT_STORAGE_BLOBS=false +INCLUDE_ERROR_STACK_TRACES=true diff --git a/docker-compose.README.md b/docker-compose.README.md index 5dde9ba3ab..b52be8816e 100644 --- a/docker-compose.README.md +++ b/docker-compose.README.md @@ -49,16 +49,19 @@ be changed by setting the APP_PORT variable. ## Local error trace debugging The API supports conditional stack traces in JSON error responses through -`cwms.cda.api.errors.ErrorTraceSupport`. +`cwms.cda.api.errors.ExceptionTraceSupport`. Stack-trace exposure is controlled through the Togglz feature flag `INCLUDE_ERROR_STACK_TRACES`. For shared environments, enable that feature in the active `features.properties` and the API will -include traces only for authenticated users with the `CWMS User Admins` role. +include traces only for authenticated users with the `SHOW STACK TRACE` role. -For local Docker Compose or other localhost testing, requests to `localhost`, `127.0.0.1`, and -`::1` are still treated as local debug requests even when the feature is disabled. When enabled, -error responses include `details.stackTrace` along with the existing `incidentIdentifier`. +The local Docker Compose setup enables that feature by default by mounting +`./compose_files/togglz/features.properties` into the API container and passing its path through +`JAVA_OPTS`. + +In that compose environment, `l2hectest` has the `SHOW STACK TRACE` role. When enabled, error +responses include `details.stackTraceLines` along with the existing `incidentIdentifier`. Response stack traces should *not* be enabled broadly for production traffic. diff --git a/docker-compose.yml b/docker-compose.yml index b602bf2159..fea4f2c13a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -69,9 +69,10 @@ services: restart: unless-stopped volumes: - ./compose_files/pki/certs:/conf/ + - ./compose_files/togglz/features.properties:/conf/features.properties:ro - ./compose_files/tomcat/logging.properties:/usr/local/tomcat/conf/logging.properties:ro environment: - - JAVA_OPTS=-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 + - JAVA_OPTS=-Dproperties.file=/conf/features.properties -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 - CDA_JDBC_DRIVER=oracle.jdbc.driver.OracleDriver - CDA_JDBC_URL=jdbc:oracle:thin:@db/FREEPDB1 - CDA_JDBC_USERNAME=q0webtest From 1b4c183b2c2c09377f105da9fa42343344599a08 Mon Sep 17 00:00:00 2001 From: "Charles Graham, SWT" Date: Tue, 21 Apr 2026 17:42:18 -0500 Subject: [PATCH 10/10] Restore tsid controller error import --- .../cwms/cda/api/TimeSeriesIdentifierDescriptorController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java index 63c6d2591e..fcd567cb61 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java @@ -30,6 +30,7 @@ import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; +import cwms.cda.api.errors.CdaError; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.TimeSeriesIdentifierDescriptorDao; import cwms.cda.data.dto.TimeSeriesIdentifierDescriptor;