Skip to content

Commit 3ebc388

Browse files
committed
COLDBOX-1400 #resolve
root appender could be called before initialization of logbox in loading
1 parent d614491 commit 3ebc388

3 files changed

Lines changed: 138 additions & 3 deletions

File tree

system/logging/LogBox.cfc

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,19 @@ component accessors="true" {
166166
variables.appenderRegistry = structNew();
167167
variables.loggerRegistry = structNew();
168168

169+
// Sentinel ROOT logger: prevents KeyNotFoundException when appender
170+
// constructors (e.g. custom Sentry/ELK appenders) pull a logger via
171+
// DI/interceptors during registerAppender() mid-configure.
172+
// levelMax=5 (OFF) so any log calls during the configure window are silent.
173+
var sentinelRoot = new coldbox.system.logging.Logger(
174+
category : "ROOT",
175+
levelMin : 0,
176+
levelMax : 5,
177+
appenders : {},
178+
serializeExtraInfo : false
179+
);
180+
variables.loggerRegistry[ "ROOT" ] = sentinelRoot;
181+
169182
// Get appender definitions
170183
var appenders = variables.config.getAllAppenders();
171184

@@ -176,7 +189,7 @@ component accessors="true" {
176189

177190
// Get Root def
178191
var rootConfig = variables.config.getRoot();
179-
// Create Root Logger
192+
// Create Root Logger replacing the sentinel, now that appenders are populated
180193
var args = {
181194
category : "ROOT",
182195
levelMin : rootConfig.levelMin,
@@ -185,8 +198,8 @@ component accessors="true" {
185198
serializeExtraInfo : variables.config.getSerializeExtraInfo()
186199
};
187200

188-
// Save in Registry
189-
variables.loggerRegistry = { "ROOT" : new coldbox.system.logging.Logger( argumentCollection = args ) };
201+
// Save in Registry, replacing the sentinel
202+
variables.loggerRegistry[ "ROOT" ] = new coldbox.system.logging.Logger( argumentCollection = args );
190203
}
191204

192205
/**
@@ -215,6 +228,18 @@ component accessors="true" {
215228
* @return coldbox.system.logging.Logger
216229
*/
217230
function getRootLogger(){
231+
// Defensive fallback: if registry is transiently empty (edge-case),
232+
// return a silent transient Logger so callers don't crash.
233+
// Constructed directly via `new` to avoid DI/logBox recursion.
234+
if ( !structKeyExists( variables.loggerRegistry, "ROOT" ) ) {
235+
return new coldbox.system.logging.Logger(
236+
category : "ROOT",
237+
levelMin : 0,
238+
levelMax : 5,
239+
appenders : {},
240+
serializeExtraInfo : false
241+
);
242+
}
218243
return variables.loggerRegistry[ "ROOT" ];
219244
}
220245

tests/specs/logging/LogBoxEdgeCases.cfc

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,83 @@ component extends="testbox.system.BaseSpec" {
1919
} );
2020
} );
2121
} );
22+
23+
story( "I want to verify LogBox survives appender constructor triggering logger access during configure", function(){
24+
given( "A custom appender whose onRegistration() calls getRootLogger()", function(){
25+
then( "it should construct without KeyNotFoundException", function(){
26+
var config = {
27+
appenders : {
28+
sentinelAppender : {
29+
class : "tests.specs.logging.appenders.SentinelTestAppender",
30+
properties : {},
31+
levelMin : 0,
32+
levelMax : 4
33+
}
34+
},
35+
root : { levelMax: "INFO", appenders : "sentinelAppender" }
36+
};
37+
38+
// This constructor calls configure() which triggers registerAppender,
39+
// which chains onRegistration() which calls getRootLogger().
40+
// Without the sentinel fix, this throws KeyNotFoundException.
41+
var logBox = new coldbox.system.logging.LogBox( config );
42+
43+
// Verify getRootLogger returns a real Logger after construction
44+
var root = logBox.getRootLogger();
45+
expect( root ).toBeComponent();
46+
expect( root.getCategory() ).toBe( "ROOT" );
47+
48+
// Verify the sentinel appender captured a non-null root logger
49+
var appender = logBox.getAppenderRegistry()[ "sentinelAppender" ];
50+
expect( appender.getCapturedRootLogger() ).toBeComponent();
51+
} );
52+
} );
53+
} );
54+
55+
story( "I want to re-configure LogBox and verify getRootLogger never returns empty", function(){
56+
given( "An already-constructed LogBox", function(){
57+
then( "calling configure() again should not break getRootLogger()", function(){
58+
var config = {
59+
appenders : {
60+
consoleAppender : {
61+
class : "ConsoleAppender",
62+
properties : {},
63+
levelMin : 0,
64+
levelMax : 4
65+
}
66+
},
67+
root : { levelMax: "INFO", appenders : "consoleAppender" }
68+
};
69+
70+
var logBox = new coldbox.system.logging.LogBox( config );
71+
expect( logBox.getRootLogger() ).toBeComponent();
72+
73+
// Re-configure: this resets registries and re-runs the appender loop.
74+
// The sentinel root must be in place during the entire re-configure.
75+
logBox.configure( config );
76+
77+
expect( logBox.getRootLogger() ).toBeComponent();
78+
expect( logBox.getRootLogger().getCategory() ).toBe( "ROOT" );
79+
} );
80+
} );
81+
} );
82+
83+
story( "I want getRootLogger() to have a defensive fallback when registry is empty", function(){
84+
given( "A LogBox with the ROOT key manually removed from its loggerRegistry", function(){
85+
then( "it should return a transient Logger instead of throwing", function(){
86+
var config = new coldbox.system.logging.config.LogBoxConfig();
87+
var logBox = new coldbox.system.logging.LogBox( config );
88+
89+
// Simulate a corrupt/empty registry state (extreme edge case)
90+
structDelete( logBox.getLoggerRegistry(), "ROOT" );
91+
92+
// Should return a Logger, not throw KeyNotFoundException
93+
var root = logBox.getRootLogger();
94+
expect( root ).toBeComponent();
95+
expect( root.getCategory() ).toBe( "ROOT" );
96+
} );
97+
} );
98+
} );
2299
} );
23100
}
24101

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Test appender for verifying the sentinel ROOT logger fix.
3+
* Overrides onRegistration() to call getRootLogger() — this simulates
4+
* the race condition where an appender triggers logger access during
5+
* LogBox.configure()'s registerAppender loop, before ROOT has been
6+
* fully re-inserted into the loggerRegistry.
7+
*
8+
* When the sentinel is in place, getRootLogger() returns the sentinel.
9+
* Without the fix, this throws KeyNotFoundException: ROOT.
10+
*/
11+
component extends="coldbox.system.logging.AbstractAppender" {
12+
13+
/**
14+
* Override onRegistration to access the root logger mid-configure.
15+
* This reproduces the crash pattern: custom appenders (like Sentry)
16+
* that pull DI/interceptors in their constructor, which eventually
17+
* calls getRootLogger() while configure() is still mid-loop.
18+
*/
19+
function onRegistration(){
20+
// Access the root logger via the LogBox reference (set by registerAppender chain)
21+
// This would throw KeyNotFoundException without the sentinel fix
22+
variables.rootLoggerCheck = getLogBox().getRootLogger();
23+
return this;
24+
}
25+
26+
/**
27+
* Expose the captured root logger for assertions.
28+
*/
29+
function getCapturedRootLogger(){
30+
return variables.rootLoggerCheck;
31+
}
32+
33+
}

0 commit comments

Comments
 (0)