Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 94 additions & 4 deletions grails-core/src/main/groovy/grails/util/GrailsUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.beans.BeanUtils;

import grails.config.Config;
import grails.config.Settings;
import grails.core.GrailsApplication;
import org.grails.exceptions.reporting.DefaultStackTraceFilterer;
import org.grails.exceptions.reporting.StackTraceFilterer;

Expand All @@ -36,7 +41,15 @@ public class GrailsUtil {

private static final Log LOG = LogFactory.getLog(GrailsUtil.class);
private static final boolean LOG_DEPRECATED = Boolean.valueOf(System.getProperty("grails.log.deprecated", String.valueOf(Environment.isDevelopmentMode())));
private static final StackTraceFilterer stackFilterer = new DefaultStackTraceFilterer();

/**
* Lazily-resolved filterer used by {@link #printSanitizedStackTrace}, {@link #sanitizeRootCause}
* and {@link #deepSanitize}. Cached once a {@link GrailsApplication} is discoverable via
* {@link Holders#findApplication()} so the config-driven class and emission flag are read
* exactly once. Volatile to publish the cached value safely; double-checked init in
* {@link #resolveStackFilterer()}.
*/
private static volatile StackTraceFilterer stackFilterer;

private GrailsUtil() {
}
Expand Down Expand Up @@ -106,7 +119,7 @@ public static void warn(String message) {
}

public static void printSanitizedStackTrace(Throwable t, PrintWriter p) {
printSanitizedStackTrace(t, p, stackFilterer);
printSanitizedStackTrace(t, p, resolveStackFilterer());
}

public static void printSanitizedStackTrace(Throwable t, PrintWriter p, StackTraceFilterer stackTraceFilterer) {
Expand Down Expand Up @@ -144,7 +157,7 @@ public static Throwable extractRootCause(Throwable t) {
* @return The root cause exception instance, with its stace trace modified to filter out grails runtime classes
*/
public static Throwable sanitizeRootCause(Throwable t) {
return stackFilterer.filter(extractRootCause(t));
return resolveStackFilterer().filter(extractRootCause(t));
}

/**
Expand All @@ -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();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Before it always used one instance, but now it's using multiple potentially. This seems backwards.

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();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What scenario is this occuring? Now we're initializing this every access until the app is online. Why aren't we just setting this value earlier in the bootstrap context and then referencing outside of the bean scope?

}
synchronized (GrailsUtil.class) {
cached = stackFilterer;
if (cached != null) {
return cached;
}
stackFilterer = createConfiguredFilterer(application);
return stackFilterer;
}
}

private static GrailsApplication findApplicationQuietly() {
try {
return Holders.findApplication();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this throws it's likely trying to fetch this prior to the application being initialized. It seems like this would be a major issue because before this functioned regardless of the application being initialized.

}
catch (Throwable ignored) {
return null;
}
}

private static StackTraceFilterer createConfiguredFilterer(GrailsApplication application) {
Class<? extends StackTraceFilterer> filtererClass = DefaultStackTraceFilterer.class;
boolean logOnFilter = true;
try {
Config config = application.getConfig();
if (config != null) {
filtererClass = config.getProperty(
Settings.SETTING_LOGGING_STACKTRACE_FILTER_CLASS,
Class.class, DefaultStackTraceFilterer.class);
logOnFilter = config.getProperty(
Settings.SETTING_LOG_FULL_STACKTRACE_ON_FILTER,
Boolean.class, true);
}
}
catch (Throwable t) {
LOG.warn("Unable to resolve StackTraceFilterer config; using default: " + t.getMessage());
}
StackTraceFilterer instance;
try {
instance = BeanUtils.instantiateClass(filtererClass, StackTraceFilterer.class);
}
catch (Throwable t) {
LOG.warn("Problem instantiating configured StackTraceFilterer [" + filtererClass.getName() +
"], falling back to default: " + t.getMessage());
instance = new DefaultStackTraceFilterer();
}
if (instance instanceof DefaultStackTraceFilterer) {
((DefaultStackTraceFilterer) instance).setLogFullStackTraceOnFilter(logOnFilter);
}
return instance;
}

}
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you point me to an integration test that exercises this logic?


GrailsApplication previousApplication

def setup() {
previousApplication = Holders.findApplication()
resetCachedFilterer()
}

def cleanup() {
Holders.setGrailsApplication(previousApplication)
resetCachedFilterer()
}

def 'falls back to a DefaultStackTraceFilterer when no GrailsApplication is discoverable'() {
given:
Holders.setGrailsApplication(null)

when:
def ex = new RuntimeException('boom')
GrailsUtil.deepSanitize(ex)

then:
noExceptionThrown()
}

def 'honours grails.logging.stackTraceFiltererClass'() {
given:
def application = Mock(GrailsApplication)
def config = Mock(Config)
config.getProperty('grails.logging.stackTraceFiltererClass', Class, DefaultStackTraceFilterer) >> RecordingStackTraceFilterer
config.getProperty('grails.exceptionresolver.logFullStackTraceOnFilter', Boolean, true) >> true
application.getConfig() >> config
Holders.setGrailsApplication(application)

when:
def ex = new RuntimeException('boom')
GrailsUtil.deepSanitize(ex)

then:
RecordingStackTraceFilterer.lastInstance != null
RecordingStackTraceFilterer.lastInstance.recursiveCalls == 1
}

def 'propagates logFullStackTraceOnFilter to DefaultStackTraceFilterer instances'() {
given:
def application = Mock(GrailsApplication)
def config = Mock(Config)
config.getProperty('grails.logging.stackTraceFiltererClass', Class, DefaultStackTraceFilterer) >> DefaultStackTraceFilterer
config.getProperty('grails.exceptionresolver.logFullStackTraceOnFilter', Boolean, true) >> false
application.getConfig() >> config
Holders.setGrailsApplication(application)

and: 'captured StackTrace logger output'
def originalErr = System.err
def baos = new ByteArrayOutputStream()
System.setErr(new PrintStream(baos, true))

when:
GrailsUtil.deepSanitize(new RuntimeException('boom'))

then:
System.err.flush()
!baos.toString().contains('ERROR StackTrace')

cleanup:
System.setErr(originalErr)
}

private static void resetCachedFilterer() {
def field = GrailsUtil.getDeclaredField('stackFilterer')
field.accessible = true
field.set(null, null)
}

static class RecordingStackTraceFilterer implements StackTraceFilterer {
static RecordingStackTraceFilterer lastInstance
int singleCalls = 0
int recursiveCalls = 0

RecordingStackTraceFilterer() {
lastInstance = this
}

Throwable filter(Throwable source) {
singleCalls++
return source
}

Throwable filter(Throwable source, boolean recursive) {
recursiveCalls++
return source
}

void addInternalPackage(String name) {}
void setCutOffPackage(String cutOffPackage) {}
void setShouldFilter(boolean shouldFilter) {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ log record. It means non-resolver code paths (for example, a scheduled job that
`GrailsUtil.sanitizeRootCause(ex)` before logging via its own logger) continue to populate the `StackTrace`
appender without an explicit emission call.

NOTE: `GrailsUtil` resolves its filterer lazily from the same config keys as the exception resolver
(`grails.logging.stackTraceFiltererClass` and `grails.exceptionresolver.logFullStackTraceOnFilter`),
so this property controls both resolver-driven _and_ `GrailsUtil`-driven emission (including the GSP
view-render path through `GroovyPageView.handleException`). Custom `StackTraceFilterer` implementations
that don't extend `DefaultStackTraceFilterer` are responsible for their own logging policy.

The behaviour is enabled by default. To disable the side-effect emission and rely solely on
`logFullStackTrace` for resolver-driven output, set:

Expand Down
8 changes: 8 additions & 0 deletions grails-doc/src/en/guide/upgrading/upgrading71x.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -849,3 +849,11 @@ Set to `false` to disable the side-effect emission and rely solely on `logFullSt
output. The two flags interact — if both are enabled, a request exception with N causes produces N+1 `StackTrace`
records (one resolver-driven plus one per throwable visited by the recursive filter walk). The Logging Full
Stack Traces section of the user guide includes a matrix of behaviours for the four flag combinations.

`GrailsUtil` honours both `grails.logging.stackTraceFiltererClass` and
`grails.exceptionresolver.logFullStackTraceOnFilter` as well — the filterer is resolved lazily on first use
from the application config, so non-resolver paths (including GSP view-render exceptions routed through
`GroovyPageView.handleException` → `GrailsUtil.deepSanitize`) participate in the same emission policy as
the resolver. Pre-7.1, `GrailsUtil` held a hardcoded `DefaultStackTraceFilterer` static field that ignored
both keys; applications that previously had to silence the `StackTrace` logger in logback purely to suppress
GSP-render-time noise can now set `logFullStackTraceOnFilter: false` and reach every caller of the filterer.
Loading