jdaugherty commented on code in PR #15666:
URL: https://github.com/apache/grails-core/pull/15666#discussion_r3652774175
##########
grails-core/src/main/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializer.java:
##########
@@ -62,5 +78,60 @@ public void initialize(BootstrapRegistry registry) {
.registerSingleton(PluginDiscovery.BEAN_NAME, discovery);
LOG.debug("Promoted GrailsPluginDiscovery to ApplicationContext as
'{}'", PluginDiscovery.BEAN_NAME);
});
+
+ // Resolve the configured StackTraceFilterer from the environment
(same two keys
+ // GrailsExceptionResolver honours) and install + promote it the same
way, before refresh().
+ registry.addCloseListener(event -> {
+ ConfigurableApplicationContext applicationContext =
event.getApplicationContext();
+ StackTraceFilterer filterer =
resolveConfiguredStackTraceFilterer(applicationContext.getEnvironment());
+ GrailsUtil.initializeStackFilterer(filterer);
+ applicationContext.getBeanFactory()
+ .registerSingleton(StackTraceFilterer.BEAN_NAME, filterer);
+ LOG.debug("Promoted StackTraceFilterer to ApplicationContext as
'{}'", StackTraceFilterer.BEAN_NAME);
+ });
+ }
+
+ /**
+ * Resolves a {@link StackTraceFilterer} from the given environment,
honouring
+ * {@link Settings#SETTING_LOGGING_STACKTRACE_FILTER_CLASS} and
+ * {@link Settings#SETTING_LOG_FULL_STACKTRACE_ON_FILTER}.
+ *
+ * <p>Resolves the class name manually via {@link ClassUtils#forName}
rather than
+ * {@code environment.getProperty(key, Class.class)} — neither Spring's
default conversion
+ * service nor Spring Boot's {@code ApplicationConversionService} register
a String-to-Class
+ * converter, so that call would throw {@code ConverterNotFoundException}
for a real class name.
+ *
+ * <p>Defensive: any failure reading config or instantiating the
configured class falls back to
+ * a plain {@link DefaultStackTraceFilterer}, matching the resolver's own
fallback behaviour.
+ */
+ private StackTraceFilterer resolveConfiguredStackTraceFilterer(Environment
environment) {
+ Class<? extends StackTraceFilterer> filtererClass =
DefaultStackTraceFilterer.class;
+ String configuredClassName =
environment.getProperty(Settings.SETTING_LOGGING_STACKTRACE_FILTER_CLASS);
+ if (StringUtils.hasText(configuredClassName)) {
+ try {
+ filtererClass = ClassUtils.forName(configuredClassName,
getClass().getClassLoader())
Review Comment:
`getClass()` is `GrailsBootstrapRegistryInitializer`, loaded from the
grails-core jar, so this resolves against the jar's ClassLoader.
Under `spring-boot-devtools` -- which grails-forge adds to generated
applications as a `developmentOnly` dependency -- application classes are
loaded by `RestartClassLoader` while jar classes stay on the base loader. A
custom filterer living in `grails-app` or `src/main/groovy` is therefore
invisible from here, and every dev-mode restart warns and silently falls back
to the default. That is precisely the environment where someone is tuning
stack-trace noise.
`postProcessApplicationContext()` runs before
`bootstrapContext.close(context)` in `SpringApplication.prepareContext`, so the
context's own loader is already set at this point:
```java
ClassUtils.forName(configuredClassName, applicationContext.getClassLoader())
```
That would need the listener to pass the context (or its ClassLoader) into
this method alongside the environment.
##########
grails-core/src/main/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializer.java:
##########
@@ -62,5 +78,60 @@ public void initialize(BootstrapRegistry registry) {
.registerSingleton(PluginDiscovery.BEAN_NAME, discovery);
LOG.debug("Promoted GrailsPluginDiscovery to ApplicationContext as
'{}'", PluginDiscovery.BEAN_NAME);
});
+
+ // Resolve the configured StackTraceFilterer from the environment
(same two keys
+ // GrailsExceptionResolver honours) and install + promote it the same
way, before refresh().
+ registry.addCloseListener(event -> {
+ ConfigurableApplicationContext applicationContext =
event.getApplicationContext();
+ StackTraceFilterer filterer =
resolveConfiguredStackTraceFilterer(applicationContext.getEnvironment());
+ GrailsUtil.initializeStackFilterer(filterer);
+ applicationContext.getBeanFactory()
+ .registerSingleton(StackTraceFilterer.BEAN_NAME, filterer);
+ LOG.debug("Promoted StackTraceFilterer to ApplicationContext as
'{}'", StackTraceFilterer.BEAN_NAME);
+ });
+ }
+
+ /**
+ * Resolves a {@link StackTraceFilterer} from the given environment,
honouring
+ * {@link Settings#SETTING_LOGGING_STACKTRACE_FILTER_CLASS} and
+ * {@link Settings#SETTING_LOG_FULL_STACKTRACE_ON_FILTER}.
+ *
+ * <p>Resolves the class name manually via {@link ClassUtils#forName}
rather than
+ * {@code environment.getProperty(key, Class.class)} — neither Spring's
default conversion
+ * service nor Spring Boot's {@code ApplicationConversionService} register
a String-to-Class
+ * converter, so that call would throw {@code ConverterNotFoundException}
for a real class name.
+ *
+ * <p>Defensive: any failure reading config or instantiating the
configured class falls back to
+ * a plain {@link DefaultStackTraceFilterer}, matching the resolver's own
fallback behaviour.
+ */
+ private StackTraceFilterer resolveConfiguredStackTraceFilterer(Environment
environment) {
+ Class<? extends StackTraceFilterer> filtererClass =
DefaultStackTraceFilterer.class;
+ String configuredClassName =
environment.getProperty(Settings.SETTING_LOGGING_STACKTRACE_FILTER_CLASS);
Review Comment:
Reading this key as a `String` drops the configuration form this PR was
opened to fix.
`application.groovy` is loaded by `GroovyConfigPropertySourceLoader` into a
`NavigableMapPropertySource`, which returns values with their original types.
So the form in the description --
```groovy
grails.logging.stackTraceFiltererClass =
com.pixoto.grails.NonLoggingStackTraceFilterer.class
```
-- reaches the environment as a `java.lang.Class`.
`environment.getProperty(key)` asks for `String.class`, Spring falls back to
`Object#toString()`, and the value becomes `"class
com.pixoto.grails.NonLoggingStackTraceFilterer"`. `ClassUtils.forName` then
fails, the code warns, and installs the default.
That is a regression rather than a no-op, because `createStackFilterer()`
now short-circuits on the promoted bean: the resolver loses the custom filterer
too. Before this PR it resolved correctly -- `PropertySourcesConfig` keeps the
raw value and `NavigableMapConfig.convertValueIfNecessary` returns it through
the `targetType.isInstance(originalValue)` branch.
Reading the raw value and accepting both shapes covers it:
```java
Object configured =
environment.getProperty(Settings.SETTING_LOGGING_STACKTRACE_FILTER_CLASS,
Object.class);
if (configured instanceof Class<?> configuredClass) {
filtererClass = configuredClass.asSubclass(StackTraceFilterer.class);
}
else if (configured instanceof CharSequence configuredName &&
StringUtils.hasText(configuredName)) {
filtererClass = ClassUtils.forName(configuredName.toString(),
classLoader)
.asSubclass(StackTraceFilterer.class);
}
```
Worth noting the flip side, since it argues for keeping both paths: the YAML
string form never worked before this PR either -- `Config.getProperty(key,
Class.class, ...)` swallows the `ConverterNotFoundException` and silently
returns the default -- so the string handling here is a real fix. It just
cannot come at the cost of the `Class` form.
##########
grails-bootstrap/src/main/groovy/org/grails/exceptions/reporting/StackTraceFilterer.java:
##########
@@ -35,6 +35,16 @@ public interface StackTraceFilterer {
String FULL_STACK_TRACE_MESSAGE = "Full Stack Trace:";
String SYS_PROP_DISPLAY_FULL_STACKTRACE = "grails.full.stacktrace";
+ /**
+ * Name under which the {@link
org.apache.grails.core.GrailsBootstrapRegistryInitializer}
+ * promotes the config-resolved filterer as an {@code ApplicationContext}
singleton bean,
+ * so later-lifecycle consumers (e.g. {@code GrailsExceptionResolver})
reuse the same
+ * instance instead of instantiating a second copy from config.
+ *
+ * @since 8.0
+ */
+ String BEAN_NAME = "stackTraceFilterer";
Review Comment:
This constant is in the wrong module. grails-bootstrap has no dependency on
grails-core -- the dependency runs the other way -- so the `{@link
org.apache.grails.core.GrailsBootstrapRegistryInitializer}` on line 39 cannot
resolve, and a Spring bean-name constant sitting in the module that
deliberately stays out of Spring wiring inverts the layering.
`PluginDiscovery.BEAN_NAME`, the pattern this follows, lives in grails-core
right next to the registrar that promotes it. Suggest moving `BEAN_NAME` to
grails-core (`GrailsBootstrapRegistryInitializer` itself is a reasonable home)
so the constant sits with the code that registers the bean; at minimum the
cross-module `{@link}` should become `{@code}`.
Unrelated drive-by while you are in this area: `GrailsConsole.java:429`
still says "can't use `StackTraceFilterer#SYS_PROP_DISPLAY_FULL_STACKTRACE` as
it is in grails-core", which is stale now that the interface lives here.
##########
grails-core/src/main/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializer.java:
##########
@@ -62,5 +78,60 @@ public void initialize(BootstrapRegistry registry) {
.registerSingleton(PluginDiscovery.BEAN_NAME, discovery);
LOG.debug("Promoted GrailsPluginDiscovery to ApplicationContext as
'{}'", PluginDiscovery.BEAN_NAME);
});
+
+ // Resolve the configured StackTraceFilterer from the environment
(same two keys
+ // GrailsExceptionResolver honours) and install + promote it the same
way, before refresh().
+ registry.addCloseListener(event -> {
+ ConfigurableApplicationContext applicationContext =
event.getApplicationContext();
+ StackTraceFilterer filterer =
resolveConfiguredStackTraceFilterer(applicationContext.getEnvironment());
+ GrailsUtil.initializeStackFilterer(filterer);
+ applicationContext.getBeanFactory()
+ .registerSingleton(StackTraceFilterer.BEAN_NAME, filterer);
+ LOG.debug("Promoted StackTraceFilterer to ApplicationContext as
'{}'", StackTraceFilterer.BEAN_NAME);
+ });
+ }
+
+ /**
+ * Resolves a {@link StackTraceFilterer} from the given environment,
honouring
+ * {@link Settings#SETTING_LOGGING_STACKTRACE_FILTER_CLASS} and
+ * {@link Settings#SETTING_LOG_FULL_STACKTRACE_ON_FILTER}.
+ *
+ * <p>Resolves the class name manually via {@link ClassUtils#forName}
rather than
+ * {@code environment.getProperty(key, Class.class)} — neither Spring's
default conversion
+ * service nor Spring Boot's {@code ApplicationConversionService} register
a String-to-Class
+ * converter, so that call would throw {@code ConverterNotFoundException}
for a real class name.
+ *
+ * <p>Defensive: any failure reading config or instantiating the
configured class falls back to
Review Comment:
The code is not as defensive as this paragraph claims. Only
`ClassUtils.forName` and `BeanUtils.instantiateClass` sit inside a `try`; both
`environment.getProperty(...)` calls -- and the close-listener body itself --
are unguarded.
So a value that fails conversion propagates out of
`bootstrapContext.close()` and fails application startup.
`logFullStackTraceOnFilter: yes-please` is enough to trigger it via the
`Boolean.class` read on line 120, and a `Class`-valued property would do it on
line 109 if the environment's conversion service has no path to `String`. The
old resolver path degraded to the default in both cases, because
`NavigableMapConfig.convertValueIfNecessary` catches `ConversionException`.
Please wrap the property reads (or the whole listener body) so a bad config
value cannot take the application down -- a filterer misconfiguration should
never be fatal.
##########
grails-core/src/test/groovy/grails/util/GrailsUtilStackFiltererSpec.groovy:
##########
@@ -0,0 +1,173 @@
+/*
+ * 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 org.grails.exceptions.reporting.DefaultStackTraceFilterer
+import org.grails.exceptions.reporting.StackTraceFilterer
+import spock.lang.Specification
+
+import java.lang.reflect.Field
+
+/**
+ * Verifies that {@link
GrailsUtil#initializeStackFilterer(StackTraceFilterer)} installs the given
+ * filterer for {@link GrailsUtil#deepSanitize}, {@link
GrailsUtil#sanitizeRootCause} and
+ * {@link GrailsUtil#printSanitizedStackTrace}, and that the
pre-initialization fallback (a
+ * {@link DefaultStackTraceFilterer}) is used until then. Config-driven
resolution (the configured
+ * class + {@code logFullStackTraceOnFilter}) now happens in
+ * {@code org.apache.grails.core.GrailsBootstrapRegistryInitializer}, covered
separately by
+ * {@code GrailsBootstrapRegistryInitializerSpec}.
+ */
+class GrailsUtilStackFiltererSpec extends Specification {
+
+ StackTraceFilterer previous
+
+ def setup() {
+ previous = currentFilterer()
+ setFilterer(fallbackFilterer())
+ }
+
+ def cleanup() {
+ setFilterer(previous)
+ }
+
+ def 'deepSanitize does not throw before initializeStackFilterer is
called'() {
+ when:
+ GrailsUtil.deepSanitize(new RuntimeException('boom'))
+
+ then:
+ noExceptionThrown()
+ }
+
+ def 'initializeStackFilterer is a no-op when filterer is null'() {
+ when:
+ GrailsUtil.initializeStackFilterer(null)
+
+ then:
+ currentFilterer().is(fallbackFilterer())
+ }
+
+ def 'initializeStackFilterer installs the given filterer'() {
+ given:
+ def filterer = new RecordingStackTraceFilterer()
+
+ when:
+ GrailsUtil.initializeStackFilterer(filterer)
+ GrailsUtil.deepSanitize(new RuntimeException('boom'))
+
+ then:
+ currentFilterer().is(filterer)
+ filterer.recursiveCalls == 1
+ }
+
+ def 'last initializeStackFilterer call wins when invoked more than once'()
{
+ given:
+ def first = new RecordingStackTraceFilterer()
+ def second = new RecordingStackTraceFilterer()
+
+ when:
+ GrailsUtil.initializeStackFilterer(first)
+ GrailsUtil.initializeStackFilterer(second)
+
+ then:
+ currentFilterer().is(second)
+ }
+
+ def 'installed DefaultStackTraceFilterer honours
logFullStackTraceOnFilter=false'() {
+ given: 'captured System.err'
+ def originalErr = System.err
+ def baos = new ByteArrayOutputStream()
+ System.setErr(new PrintStream(baos, true))
+
+ and: 'a filterer with the side-effect emission disabled'
+ def quietFilterer = new DefaultStackTraceFilterer()
+ quietFilterer.logFullStackTraceOnFilter = false
+
+ when:
+ GrailsUtil.initializeStackFilterer(quietFilterer)
+ GrailsUtil.deepSanitize(exceptionWithApplicationFrame())
+
+ then: "no 'Full Stack Trace:' entry is emitted"
+ System.err.flush()
+ !baos.toString().contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE)
+
+ cleanup:
+ System.setErr(originalErr)
+ }
+
+ def 'installed DefaultStackTraceFilterer emits Full Stack Trace by
default'() {
+ given: 'captured System.err'
+ def originalErr = System.err
+ def baos = new ByteArrayOutputStream()
+ System.setErr(new PrintStream(baos, true))
+
+ and: 'a filterer with the default (enabled) side-effect emission'
+ def loudFilterer = new DefaultStackTraceFilterer()
+
+ when:
+ GrailsUtil.initializeStackFilterer(loudFilterer)
+ GrailsUtil.deepSanitize(exceptionWithApplicationFrame())
+
+ then: "a 'Full Stack Trace:' entry is emitted -- the positive control
proving the negative case above is meaningful"
+ System.err.flush()
+ baos.toString().contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE)
+
+ cleanup:
+ System.setErr(originalErr)
+ }
+
+ private static RuntimeException exceptionWithApplicationFrame() {
+ def exception = new RuntimeException('boom')
+ exception.stackTrace = [
+ new StackTraceElement('test.FooController', 'show',
'FooController.groovy', 6),
+ new StackTraceElement('java.lang.reflect.Method', 'invoke',
'Method.java', 580)
+ ] as StackTraceElement[]
+ exception
+ }
+
+ private static StackTraceFilterer currentFilterer() {
+ filtererField().get(null) as StackTraceFilterer
+ }
+
+ private static void setFilterer(StackTraceFilterer filterer) {
+ filtererField().set(null, filterer)
+ }
+
+ private static StackTraceFilterer fallbackFilterer() {
+ Field field = GrailsUtil.getDeclaredField('FALLBACK_FILTERER')
Review Comment:
These specs reach into private statics by reflection (`stackFilterer`,
`FALLBACK_FILTERER`) and then assert on them -- `currentFilterer().is(bean)`
and friends. `CLAUDE.md` rule 9 asks tests to exercise behaviour through the
same surface an end user calls, and every assertion here has a behavioural
equivalent: install a recording filterer and assert it was invoked via
`GrailsUtil.deepSanitize`, which `'initializeStackFilterer installs the given
filterer'` already does well.
Save/restore of the static across features is fair enough given there is no
public reset, but the *assertions* should not be white-box -- as written they
also couple the specs to field names that are otherwise free to change. Same
comment applies to `GrailsBootstrapRegistryInitializerSpec`.
##########
grails-web-mvc/src/main/groovy/org/grails/web/errors/GrailsExceptionResolver.java:
##########
@@ -453,6 +460,28 @@ protected void createStackFilterer() {
applyLogFullStackTraceOnFilter();
}
+ /**
+ * Looks up the {@link StackTraceFilterer} that
+ * {@link org.apache.grails.core.GrailsBootstrapRegistryInitializer}
promoted to the
+ * {@code ApplicationContext} during bootstrap, so this resolver reuses
that instance instead
+ * of instantiating a second copy from config. Returns {@code null} when
no such bean is
+ * registered — e.g. a {@code GrailsApplication} wired up outside the
normal Spring Boot
+ * bootstrap sequence — in which case {@link #createStackFilterer()} falls
back to its own
+ * construction.
+ */
+ protected StackTraceFilterer resolvePromotedStackTraceFilterer() {
+ ApplicationContext context = grailsApplication.getMainContext();
+ if (context == null) {
+ return null;
+ }
+ try {
+ return context.getBean(StackTraceFilterer.BEAN_NAME,
StackTraceFilterer.class);
+ }
+ catch (NoSuchBeanDefinitionException e) {
Review Comment:
Catching only `NoSuchBeanDefinitionException` narrows what this path used to
tolerate. This call happens *before* `createStackFilterer()`'s own `try`, so an
application that registers its own bean named `stackTraceFilterer` of an
unrelated type gets a `BeanNotOfRequiredTypeException` straight out of
`setGrailsApplication`, failing the context. Previously every `Throwable` in
filterer construction was contained and degraded to the default.
Either widen the catch to `BeansException` or look the bean up by type
instead of by name.
Related question on the same name collision: if an application does define
its own `stackTraceFilterer` bean, its definition is registered after the
manual singleton, so the resolver and `GrailsUtil` can end up holding different
instances. Worth a sentence in the javadoc about which one wins.
##########
grails-doc/src/en/guide/upgrading/upgrading71x.adoc:
##########
@@ -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.
+
+Both `grails.logging.stackTraceFiltererClass` and
`grails.exceptionresolver.logFullStackTraceOnFilter` now
Review Comment:
This paragraph is in the wrong chapter now that the PR targets `8.0.x`. As
written, 7.1 users are told the two keys reach `GrailsUtil`-driven paths, which
is not true on 7.1, and anyone upgrading 7.x -> 8 never sees the note at all.
`grails-doc/src/en/guide/upgrading/upgrading80x.adoc` already exists -- the
paragraph belongs there. The rest of section 2.13 describes what genuinely
shipped in 7.1 and should stay put.
##########
grails-test-examples/app2/src/integration-test/groovy/app2/GrailsUtilStackFiltererIntegrationSpec.groovy:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 app2
+
+import grails.plugin.geb.ContainerGebSpec
+import grails.testing.mixin.integration.Integration
+import org.springframework.test.context.TestPropertySource
+
+/**
+ * Boots the full application with {@code
grails.logging.stackTraceFiltererClass} set, then drives a
+ * non-resolver code path ({@link grails.util.GrailsUtil#deepSanitize}) to
prove the bootstrap wiring in
+ * {@code org.apache.grails.core.GrailsBootstrapRegistryInitializer} actually
installs the configured
+ * filterer in a real running app -- the piece {@code
GrailsUtilStackFiltererSpec} and
+ * {@code GrailsBootstrapRegistryInitializerSpec} can't prove on their own,
since they exercise the
+ * classes directly rather than through a real Spring Boot bootstrap.
+ *
+ * <p>The {@link TestPropertySource} on this class gives it a merged context
configuration distinct from
+ * {@link ErrorsControllerSpec} and {@link NotFoundHandlerSpec}, so Spring
boots a separate application
+ * context for it and the other specs in this module are unaffected by the
custom filterer class.
Review Comment:
This claim does not hold. The separate merged context isolates the *Spring*
contexts, but `GrailsUtil.stackFilterer` is a JVM-global static -- once this
spec's context boots, `RecordingStackTraceFilterer` stays installed for the
rest of the fork. `ErrorsControllerSpec` and `NotFoundHandlerSpec` have their
contexts cached by the TestContext framework and will not re-boot to overwrite
it, so depending on class ordering they run against this filterer.
Nothing fails today because neither of those specs asserts on filterer
behaviour, but the comment states the opposite of what actually happens and
will mislead whoever touches this next.
The underlying design point is worth deciding explicitly: static
installation is last-context-booted-wins, and the static also pins an instance
of an application-loaded class for the JVM lifetime (devtools restart loops
retain the old `RestartClassLoader`). Resetting to `FALLBACK_FILTERER` on
`ContextClosedEvent` would address both; otherwise the single-context
assumption should be stated in the docs. Either way please correct this
paragraph.
##########
grails-doc/src/en/guide/conf/config/logging/loggingFullStackTraces.adoc:
##########
@@ -97,6 +97,14 @@ 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: Both `grails.logging.stackTraceFiltererClass` and
`grails.exceptionresolver.logFullStackTraceOnFilter`
+apply outside the exception resolver too — including GSP view-render
exceptions — so this property controls
+emission everywhere the filterer is used, not just for resolver-driven
requests. Before the application
+context is initialized (CLI usage, tests that don't boot a context, plain
`main()`), the pre-7.1 fallback
Review Comment:
"tests that don't boot a context" is not quite the boundary -- Grails unit
tests (`ServiceUnitTest` and friends) do boot a context, they just do not go
through `SpringApplication`, so they get the fallback too. "any context not
started through `SpringApplication`" describes the actual condition without
naming internals.
Worth adding one more sentence here as well: the installed filterer is
JVM-global static state, so in a JVM hosting more than one application context
the last one booted wins. See my comment on the integration spec.
##########
grails-test-examples/app2/src/integration-test/groovy/app2/GrailsUtilStackFiltererIntegrationSpec.groovy:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 app2
+
+import grails.plugin.geb.ContainerGebSpec
+import grails.testing.mixin.integration.Integration
+import org.springframework.test.context.TestPropertySource
+
+/**
+ * Boots the full application with {@code
grails.logging.stackTraceFiltererClass} set, then drives a
+ * non-resolver code path ({@link grails.util.GrailsUtil#deepSanitize}) to
prove the bootstrap wiring in
+ * {@code org.apache.grails.core.GrailsBootstrapRegistryInitializer} actually
installs the configured
+ * filterer in a real running app -- the piece {@code
GrailsUtilStackFiltererSpec} and
+ * {@code GrailsBootstrapRegistryInitializerSpec} can't prove on their own,
since they exercise the
+ * classes directly rather than through a real Spring Boot bootstrap.
+ *
+ * <p>The {@link TestPropertySource} on this class gives it a merged context
configuration distinct from
+ * {@link ErrorsControllerSpec} and {@link NotFoundHandlerSpec}, so Spring
boots a separate application
+ * context for it and the other specs in this module are unaffected by the
custom filterer class.
+ */
+@Integration(applicationClass = Application)
+@TestPropertySource(properties =
['grails.logging.stackTraceFiltererClass=app2.RecordingStackTraceFilterer'])
Review Comment:
This exercises the string form of the key only. Given the `Class`-valued
case I flagged in `GrailsBootstrapRegistryInitializer`, please add coverage for
a filterer configured as a class literal in `application.groovy` -- that is the
shape in the original bug report and the one that currently regresses. A
unit-level case in `GrailsBootstrapRegistryInitializerSpec` (a
`MapPropertySource` holding the `Class` object rather than its name) would
catch it cheaply, without a second application context.
--
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]
