fix(module): log APIScanner class-not-found at info, not warn+stacktrace by soloturn · Pull Request #167 · MovingBlocks/gestalt

soloturn · 2026-07-30T21:05:56Z

AI-assisted change proposal. Filed by agent driven by @soloturn via GDD.

Summary

APIScanner.scan() logged a full stack trace at WARN for every module class
it couldn't resolve on the given classloader. This fires routinely — the
engine's setupSandbox() scans every registered module's class index
before that module's classes are loaded onto the classloader, so most
non-engine modules trip it on every startup. Downgraded to a one-line INFO
naming the class and the classloader checked.

Test plan

Run Terasology Latest.app and confirm startup no longer logs
ClassNotFoundException stack traces from APIScanner, just INFO lines

Summary by CodeRabbit

Bug Fixes
- Improved diagnostic logging when a class cannot be found during API scanning, including the missing class name and configured class loader.

Walkthrough

APIScanner.scan now logs ClassNotFoundException cases at info level with the missing API class and configured class loader.

Changes

API scanner logging

Layer / File(s)	Summary
Missing-class logging `gestalt-module/.../sandbox/APIScanner.java`	Missing API classes are logged at info level with the class name and configured class loader.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Poem

A rabbit logs a class not found,
With loader details neatly bound.
Info speaks softly through the air,
The scanner hops on without care.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the APIScanner logging change shown in the changeset.
Description check	✅ Passed	The description directly explains the APIScanner logging change, its cause, and the intended test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/apiscanner-info-log-not-warn

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

BenjaminAmos · 2026-07-30T22:43:01Z

This fires routinely — the engine's setupSandbox() scans every registered module's class index
before that module's classes are loaded onto the classloader, so most
non-engine modules trip it on every startup.

This seems more like the issue, rather than the warning itself. Destination Sol gets a lot of these warnings at runtime as well. If I understand correctly, is this effectively prohibiting any modules outside of the engine classpath from being exported to other modules as APIs?

soloturn · 2026-07-30T23:06:10Z

AI-assisted comment. Filed by agent driven by @soloturn via GDD.

@BenjaminAmos good question - I traced it through JavaModuleClassLoader.loadClass():

ClassLoader parentLoader = ObtainClassloader(clazz);
if (parentLoader != this && !(parentLoader instanceof ModuleClassLoader)) {
    if (permissionProvider.isPermitted(clazz)) {
        return clazz;
    } else {
        logger.error("Denied access...");
        return null;
    }
}
return clazz;  // module-to-module access: no check at all

The permission check only fires when the resolved class's own loader is neither this module's classloader nor any other ModuleClassLoader - i.e. it only gates access to engine/system-classloader classes. Any class whose loader is a ModuleClassLoader (any other module, including a dependency) returns unchecked. So module-to-module API exports aren't gated by this mechanism at all - DestinationSol's own APIs aren't blocked by it.

What the warning storm actually is: ModuleManager.setupSandbox() loops over every registered module and calls apiScanner.scan(module.getClassIndex()) against the system classloader - including modules whose classes were never going to be checked against this permission set in the first place. So it's dead work, not a real error, but you're right that the scan loop is the actual issue, not just the log level.

Filed the underlying fix separately since setupSandbox() lives in the engine repo, not here: MovingBlocks/Terasology#5353 restricts the scan to the engine module (and dev-mode classpath modules) instead of every registered module. This PR (downgrading the log level) still stands on its own regardless - even with #5353, ClassNotFoundException from other causes should log at info, not warn+stacktrace, for expected-miss cases.

soloturn · 2026-07-30T23:08:37Z

AI-assisted comment. Filed by agent driven by @soloturn via GDD.

For anyone wondering whether this PR is now redundant with MovingBlocks/Terasology#5353: it isn't, and we're keeping both.

#5353 fixes the one known call site (ModuleManager.setupSandbox()) so it stops scanning modules that were never resolvable via the system classloader - the common case goes away there.

This PR fixes APIScanner itself, which is a general-purpose method in a shared library - forClassLoader/classIndex are caller-supplied, so any other consumer (tests, other games built on gestalt, or a future Terasology call site) can still legitimately hit a mismatch. Even within Terasology, the malformed MeshBuilder.TextureMapper-with-a-dot-instead-of-$ entry visible in the original log suggests the class index can contain bad entries independent of which module is being scanned - #5353 doesn't touch that.

So this is defense-in-depth, not overlap: even after #5353 lands, any remaining ClassNotFoundException here is still the expected-miss case, not something that should dump a stack trace.

APIScanner.scan() iterates every module's class index during ModuleManager.setupSandbox(), which runs before modules are loaded onto forClassLoader - so most non-engine modules' API classes are expected to be unresolvable there, not an error. WARN + full stack trace on every one of these floods the log at startup. Downgrade to a one-line INFO stating which classloader was checked. Co-Authored-By: soloturn <soloturn@gmail.com>

soloturn · 2026-07-30T23:26:04Z

AI-assisted comment. Filed by agent driven by @soloturn via GDD.

Rebased onto current develop (was 3 commits behind), force-pushed — clean, no conflicts. Ready for review.

setupSandbox() scanned every registered module's class index against the system classloader, including modules loaded from the application path - whose classes are only ever reachable through their own ModuleClassLoader, never the system classloader. That scan was guaranteed to fail for those modules, and pointless even when it succeeded: JavaModuleClassLoader#loadClass only consults this permission set for classes loaded by a non-module classloader, so module-to-module API access was never gated by it anyway. Restrict the scan to the engine module and any dev-mode classpath modules - the only ones whose classes are actually resolvable via the system classloader setupSandbox() uses. See MovingBlocks/gestalt#167, which downgrades the resulting ClassNotFoundException logging but doesn't address the underlying scan.

Per BenjaminAmos's review on PR #167: the "expected if it belongs to a module's own classloader instead" clause assumed a specific cause for the miss that isn't always true - forClassLoader/classIndex are caller-supplied, so a mismatch can come from other call sites too. Keep the class and classloader detail, drop the assumption. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQriAKnoCEqcmFoSAvU39Q

soloturn requested a review from BenjaminAmos July 30, 2026 21:06

soloturn force-pushed the fix/apiscanner-info-log-not-warn branch from 5281d64 to 32fec58 Compare July 30, 2026 21:14

soloturn mentioned this pull request Jul 30, 2026

fix(module): stop scanning modules APIScanner can never resolve MovingBlocks/Terasology#5353

Merged

2 tasks

soloturn force-pushed the fix/apiscanner-info-log-not-warn branch from 32fec58 to 4db97e0 Compare July 30, 2026 23:13

This was referenced Jul 30, 2026

feat(ws-review): enforce GDD attribution on reply/comment, add branch-drift check soloturn/yggdrasil#1

Closed

feat(ws-review): enforce GDD attribution on reply/comment, add branch-drift check SiliconSaga/yggdrasil#141

Open

BenjaminAmos reviewed Aug 1, 2026

View reviewed changes

Comment thread gestalt-module/src/main/java/org/terasology/gestalt/module/sandbox/APIScanner.java Outdated

Navigation Menu

Uh oh!

fix(module): log APIScanner class-not-found at info, not warn+stacktrace - #167

fix(module): log APIScanner class-not-found at info, not warn+stacktrace#167
soloturn wants to merge 2 commits into
developfrom
fix/apiscanner-info-log-not-warn

soloturn commented Jul 30, 2026

Uh oh!

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

BenjaminAmos commented Jul 30, 2026

Uh oh!

soloturn commented Jul 30, 2026 •

edited

Loading

Uh oh!

soloturn commented Jul 30, 2026 •

edited

Loading

Uh oh!

soloturn commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

soloturn commented Jul 30, 2026

Summary

Test plan

Related

Uh oh!

coderabbitai Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

BenjaminAmos commented Jul 30, 2026

Uh oh!

soloturn commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soloturn commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soloturn commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading

soloturn commented Jul 30, 2026 •

edited

Loading

soloturn commented Jul 30, 2026 •

edited

Loading