codeconsole commented on code in PR #15666:
URL: https://github.com/apache/grails-core/pull/15666#discussion_r3323237677


##########
grails-core/src/main/groovy/grails/util/GrailsUtil.java:
##########
@@ -154,7 +167,84 @@ public static Throwable sanitizeRootCause(Throwable t) {
      * @return The root cause exception instances, with stack trace modified 
to filter out grails runtime classes
      */
     public static Throwable deepSanitize(Throwable t) {
-        return stackFilterer.filter(t, true);
+        return resolveStackFilterer().filter(t, true);
+    }
+
+    /**
+     * Returns the {@link StackTraceFilterer} used by this class, lazily 
initialised from the
+     * Grails application config when one is discoverable. Honours
+     * {@link Settings#SETTING_LOGGING_STACKTRACE_FILTER_CLASS} (the filterer 
class — same key
+     * the exception resolver consults) and propagates
+     * {@link Settings#SETTING_LOG_FULL_STACKTRACE_ON_FILTER} to instances of
+     * {@link DefaultStackTraceFilterer}.
+     *
+     * <p>While no {@link GrailsApplication} is available (early-init paths, 
plain {@code main}
+     * usage, tests that don't wire one up) a fresh {@link 
DefaultStackTraceFilterer} is returned
+     * and <em>not</em> cached — so once the application context boots, the 
next call resolves
+     * the configured filterer for real. After that the value is cached for 
the lifetime of the
+     * JVM, matching the historical behaviour of the previous {@code static 
final} field.
+     */
+    private static StackTraceFilterer resolveStackFilterer() {
+        StackTraceFilterer cached = stackFilterer;
+        if (cached != null) {
+            return cached;
+        }
+        GrailsApplication application = findApplicationQuietly();
+        if (application == null) {
+            // No application discoverable yet — return an uncached default. A 
later call,
+            // once the context is up, will run through the 
configured-resolution branch
+            // and populate the cache.
+            return new DefaultStackTraceFilterer();
+        }
+        synchronized (GrailsUtil.class) {
+            cached = stackFilterer;
+            if (cached != null) {
+                return cached;
+            }
+            stackFilterer = createConfiguredFilterer(application);
+            return stackFilterer;
+        }
+    }
+
+    private static GrailsApplication findApplicationQuietly() {
+        try {
+            return Holders.findApplication();

Review Comment:
   Agreed — `Holders.findApplication()` doesn't actually throw 
(Holders.java:124-132 iterates strategies and returns null), and "regardless of 
the application being initialized" is the right invariant. Switched the whole 
approach in cdffb8f3 to a bootstrap hook: 
`GrailsExceptionResolver.setGrailsApplication` calls a new public 
`GrailsUtil.initializeStackFilterer(application)` during Spring bean wiring, so 
`GrailsUtil` no longer reaches into `Holders` at all. Hot path is now a single 
volatile read.



##########
grails-core/src/test/groovy/grails/util/GrailsUtilStackFiltererSpec.groovy:
##########
@@ -0,0 +1,134 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package grails.util
+
+import grails.config.Config
+import grails.core.GrailsApplication
+import org.grails.exceptions.reporting.DefaultStackTraceFilterer
+import org.grails.exceptions.reporting.StackTraceFilterer
+import spock.lang.Specification
+
+/**
+ * Verifies that {@link GrailsUtil#deepSanitize}, {@link 
GrailsUtil#sanitizeRootCause} and
+ * {@link GrailsUtil#printSanitizedStackTrace} honour the same config keys as
+ * {@code GrailsExceptionResolver} — {@code 
grails.logging.stackTraceFiltererClass} and
+ * {@code grails.exceptionresolver.logFullStackTraceOnFilter}.
+ *
+ * The cached filterer is reset between scenarios via reflection so each test 
sees a
+ * fresh lookup against its own {@link GrailsApplication}.
+ */
+class GrailsUtilStackFiltererSpec extends Specification {

Review Comment:
   Currently only the unit spec; same coverage level the parent PR #15564 
shipped with for the resolver side of these keys. Happy to add an 
`@Integration` spec in `grails-test-examples/app2` that boots a real context, 
sets `grails.logging.stackTraceFiltererClass` in `application.yml`, calls 
`GrailsUtil.deepSanitize` from inside the running app, and asserts the 
configured class was used. Prefer `app2` (it already exercises exception 
handling) or a dedicated minimal app like `config-report`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to