Skip to content

Commit cbfb149

Browse files
Cervatorclaude
andcommitted
feat(module): report unattributable types from ReflectionUtil, throttled
The three ReflectionUtil sites were left alone in the previous commit because a null module is a documented, usually-legitimate outcome there — every non-module class loaded by the app classloader takes that branch, so reporting each one would drown the log in a path that runs per type, per serialization. But a module class that fails attribution lands in exactly the same branch and silently loses its module qualifier, which is the failure this whole effort started from. Saying nothing is the wrong trade too. So: report, throttled. ModuleAttribution.reportUnattributedOnce names at most 20 distinct classes, says so when it stops, and resets after 60s of quiet. Same shape as the discarded-write throttle in WorldProviderCoreImpl. The throttle is documented as temporary scaffolding rather than furniture — it exists to make an invisible problem visible without making it unbearable, and should go once the underlying failures are fixed. Its state is static, which is a real wart: a long test run building many engines could have one engine's reports suppressed by another's. The 60s reset bounds that. A single audit at environment switch would be better than all of this — walk the indexed classes once and report the whole set as it becomes true, instead of scattered surprises later, with no throttle needed by construction. Proposed rather than built, since it is a design change rather than a review follow-up. Verified no reports fire during a healthy engine startup (ExampleTest), so this is a fault signal rather than background noise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pXw6g6JWNGP6eAyDJ1Cf8 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8fa2e03 commit cbfb149

2 files changed

Lines changed: 68 additions & 0 deletions

File tree

‎engine/src/main/java/org/terasology/engine/core/module/ModuleAttribution.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,78 @@
33

44
package org.terasology.engine.core.module;
55

6+
import org.slf4j.Logger;
67
import org.terasology.gestalt.module.ModuleEnvironment;
78
import org.terasology.gestalt.naming.Name;
89

910
import java.util.ArrayList;
1011
import java.util.List;
12+
import java.util.Set;
13+
import java.util.concurrent.ConcurrentHashMap;
14+
import java.util.concurrent.atomic.AtomicBoolean;
1115

1216
/**
1317
* Helpers for reporting when a {@link ModuleEnvironment} cannot say which module a class came from.
1418
*/
1519
public final class ModuleAttribution {
1620

21+
/** How many distinct classes {@link #reportUnattributedOnce} names before it falls quiet. */
22+
private static final int DISTINCT_CLASS_REPORT_LIMIT = 20;
23+
24+
/**
25+
* How long the throttle stays quiet before reporting afresh.
26+
* <p>
27+
* Long enough that one startup's worth of failures counts as a single burst, short enough that
28+
* a later unrelated one is still heard.
29+
*/
30+
private static final long REPORT_RESET_MS = 60_000;
31+
32+
private static final Set<String> reportedClasses = ConcurrentHashMap.newKeySet();
33+
private static final AtomicBoolean limitAnnounced = new AtomicBoolean();
34+
private static volatile long lastReportMs;
35+
1736
private ModuleAttribution() {
1837
// static utility class, no instance needed
1938
}
2039

40+
/**
41+
* Report a class the environment cannot attribute, at most once per class and not many times.
42+
* <p>
43+
* For callers on paths hot enough that reporting every occurrence would drown the log - type
44+
* handling runs this per type, per serialization - or where a null is usually legitimate and
45+
* only occasionally a real fault. Names at most {@value #DISTINCT_CLASS_REPORT_LIMIT} distinct
46+
* classes, says so when it stops, and starts over after {@value #REPORT_RESET_MS}ms of quiet.
47+
* <p>
48+
* The throttle is deliberately crude, and deliberately static: it exists to make a
49+
* currently-invisible problem visible without making it unbearable, and should go away once
50+
* the underlying attribution failures are fixed rather than becoming permanent furniture.
51+
*
52+
* @param logger the caller's logger, so the message is attributed to where it happened
53+
* @param what a short description of what could not be done, e.g. "build a type URI"
54+
* @param type the class that could not be attributed
55+
* @param environment the environment that was asked
56+
*/
57+
public static void reportUnattributedOnce(Logger logger, String what, Class<?> type, ModuleEnvironment environment) {
58+
long now = System.currentTimeMillis();
59+
if (now - lastReportMs > REPORT_RESET_MS) {
60+
reportedClasses.clear();
61+
limitAnnounced.set(false);
62+
}
63+
lastReportMs = now;
64+
65+
if (!reportedClasses.add(type.getName())) {
66+
return;
67+
}
68+
if (reportedClasses.size() > DISTINCT_CLASS_REPORT_LIMIT) {
69+
if (limitAnnounced.compareAndSet(false, true)) {
70+
logger.warn("More than {} classes could not be attributed to a module. Further reports are"
71+
+ " suppressed until this stops happening.", DISTINCT_CLASS_REPORT_LIMIT);
72+
}
73+
return;
74+
}
75+
logger.warn("Could not {} - no module provides {}", what, describeUnattributedClass(type, environment));
76+
}
77+
2178
/**
2279
* Describe a class the environment turned up but cannot attribute to any of its modules.
2380
* <p>

‎engine/src/main/java/org/terasology/engine/utilities/ReflectionUtil.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
import com.google.common.collect.Lists;
88
import com.google.common.collect.Sets;
99
import org.reflections.ReflectionUtils;
10+
import org.slf4j.Logger;
11+
import org.slf4j.LoggerFactory;
1012
import org.terasology.engine.core.SimpleUri;
13+
import org.terasology.engine.core.module.ModuleAttribution;
1114
import org.terasology.gestalt.module.ModuleEnvironment;
1215
import org.terasology.gestalt.naming.Name;
1316
import org.terasology.nui.UIWidget;
@@ -35,6 +38,8 @@
3538

3639

3740
public final class ReflectionUtil {
41+
private static final Logger logger = LoggerFactory.getLogger(ReflectionUtil.class);
42+
3843
private ReflectionUtil() {
3944
}
4045

@@ -653,6 +658,10 @@ public static String getTypeUri(Type type, ModuleEnvironment moduleEnvironment)
653658
Name moduleProvidingType = moduleEnvironment.getModuleProviding(getRawType(type));
654659

655660
if (moduleProvidingType == null) {
661+
// Usually legitimate - plenty of non-module classes reach here - but a module class
662+
// that fails attribution lands in the same branch and silently loses its module
663+
// qualifier, so the URI stops matching. Throttled because this runs per type.
664+
ModuleAttribution.reportUnattributedOnce(logger, "build a type URI", getRawType(type), moduleEnvironment);
656665
return typeSimpleName;
657666
}
658667

@@ -674,6 +683,7 @@ public static SimpleUri getSimpleUriFor(Type type, ModuleEnvironment environment
674683
Name moduleProviding = environment.getModuleProviding(clazz);
675684

676685
if (moduleProviding == null) {
686+
ModuleAttribution.reportUnattributedOnce(logger, "build a simple URI", clazz, environment);
677687
return null;
678688
}
679689

@@ -695,6 +705,7 @@ public static SimpleUri getFullyQualifiedSimpleUriFor(Type type, ModuleEnvironme
695705
Name moduleProviding = environment.getModuleProviding(clazz);
696706

697707
if (moduleProviding == null) {
708+
ModuleAttribution.reportUnattributedOnce(logger, "build a fully qualified URI", clazz, environment);
698709
return null;
699710
}
700711

0 commit comments

Comments
 (0)