Skip to content

Provide better errors when game-layer mod files try to provide class processors #367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shartte wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Provide better errors when game-layer mod files try to provide class processors #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c55478
Provide better errors when game-layer mod files try to provide class …
shartte Oct 5, 2025
13d2358
Pre-cache the module descriptors
shartte Oct 5, 2025
ce26e77
Extract the validation into its own method.
shartte Oct 5, 2025
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
2 changes: 1 addition & 1 deletion loader/src/main/java/net/neoforged/fml/classloading/transformation/ClassProcessorSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public Builder addProcessorProviders(Collection<ClassProcessorProvider> provider
// the actual mod causing them.
var sourceFile = ServiceLoaderUtil.identifySourcePath(provider);
ModLoader.addLoadingIssue(
ModLoadingIssue.error("fml.modloadingissue.coremod_error", provider.getClass().getName(), sourceFile).withCause(e));
ModLoadingIssue.error("fml.modloadingissue.classprocessor_error", provider.getClass().getName(), sourceFile).withCause(e));
}
}
return this;
Expand Down
66 changes: 55 additions & 11 deletions loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import net.neoforged.fml.IBindingsProvider;
import net.neoforged.fml.ModList;
import net.neoforged.fml.ModLoader;
import net.neoforged.fml.ModLoadingException;
import net.neoforged.fml.ModLoadingIssue;
import net.neoforged.fml.classloading.JarModuleFinder;
import net.neoforged.fml.classloading.ResourceMaskingClassLoader;
Expand Down Expand Up @@ -141,7 +142,24 @@ record DiscoveryResult(
List<ModFile> pluginContent,
List<ModFile> gameContent,
List<ModFile> gameLibraryContent,
List<ModLoadingIssue> discoveryIssues) {}
List<ModLoadingIssue> discoveryIssues) {
public List<ModFile> allContent() {
var content = new ArrayList<ModFile>(
pluginContent.size() + gameContent.size() + gameLibraryContent.size());
content.addAll(pluginContent);
content.addAll(gameContent);
content.addAll(gameLibraryContent);
return content;
}

public List<ModFile> allGameContent() {
var content = new ArrayList<ModFile>(
gameContent.size() + gameLibraryContent.size());
content.addAll(gameContent);
content.addAll(gameLibraryContent);
return content;
}
}

private FMLLoader(ClassLoader currentClassLoader, String[] programArgs, Dist dist, boolean production, Path gameDir) {
this.currentClassLoader = currentClassLoader;
Expand Down Expand Up @@ -296,18 +314,20 @@ public static FMLLoader create(@Nullable Instrumentation instrumentation, Startu
} else {
discoveryResult = runOffThread(loader::runDiscovery);
}
ClassLoadingGuardian classLoadingGuardian = null;
if (instrumentation != null) {
classLoadingGuardian = new ClassLoadingGuardian(instrumentation, discoveryResult.gameContent);
loader.ownedResources.add(classLoadingGuardian);
}

for (var issue : discoveryResult.discoveryIssues()) {
LOGGER.atLevel(issue.severity() == ModLoadingIssue.Severity.ERROR ? Level.ERROR : Level.WARN)
.setCause(issue.cause())
.log("{}", FMLTranslations.translateIssueEnglish(issue));
}

buildModuleDescriptors(discoveryResult.allContent());

ClassLoadingGuardian classLoadingGuardian = null;
if (instrumentation != null) {
classLoadingGuardian = new ClassLoadingGuardian(instrumentation, discoveryResult.allGameContent());
loader.ownedResources.add(classLoadingGuardian);
}

var mixinFacade = new MixinFacade();
loader.ownedResources.add(mixinFacade);

Expand All @@ -323,10 +343,7 @@ public static FMLLoader create(@Nullable Instrumentation instrumentation, Startu
// BUILD GAME LAYER
// NOTE: This is where Mixin contributes its synthetic SecureJar to ensure it's generated classes are handled by the TCL
var gameContent = new ArrayList<SecureJar>();
for (var modFile : discoveryResult.gameLibraryContent) {
gameContent.add(modFile.getSecureJar());
}
for (var modFile : discoveryResult.gameContent) {
for (var modFile : discoveryResult.allGameContent()) {
gameContent.add(modFile.getSecureJar());
}

Expand Down Expand Up @@ -360,10 +377,20 @@ public static FMLLoader create(@Nullable Instrumentation instrumentation, Startu
}
}

// Build all module descriptors in parallel, since this can involve scanning for packages/services
private static void buildModuleDescriptors(List<ModFile> allContent) {
var start = System.nanoTime();
allContent.stream().parallel().forEach(mf -> mf.getSecureJar().moduleDataProvider().descriptor());
var elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
LOGGER.debug("Built module descriptors for {} mod files in {}ms", allContent.size(), elapsed);
}

private static ClassProcessorSet createClassProcessorSet(StartupArgs startupArgs,
LaunchContextAdapter launchContext,
DiscoveryResult discoveryResult,
MixinFacade mixinFacade) {
checkGameContentForClassProcessors(discoveryResult.allGameContent());

// Add our own launch plugins explicitly.
var builtInProcessors = new ArrayList<ClassProcessor>();
builtInProcessors.add(createAccessTransformerService(discoveryResult));
Expand Down Expand Up @@ -391,6 +418,23 @@ private static ClassProcessorSet createClassProcessorSet(StartupArgs startupArgs
.build();
}

// Validates, that the modder didn't try to provide transformation services from game content and gives a nice error message if they did
private static void checkGameContentForClassProcessors(List<ModFile> allGameContent) {
var issues = new ArrayList<ModLoadingIssue>();
for (var modFile : allGameContent) {
var descriptor = modFile.getSecureJar().moduleDataProvider().descriptor();
for (var provides : descriptor.provides()) {
if (provides.service().equals(ClassProcessorProvider.class.getName())
|| provides.service().equals(ClassProcessor.class.getName())) {
issues.add(ModLoadingIssue.error("fml.modloadingissue.classprocessor_in_game_content").withAffectedModFile(modFile));
Copy link
Contributor

@lukebemish lukebemish Oct 5, 2025

Choose a reason for hiding this comment

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

Don't we need to do this for all the other FML services too? Anything that requires LIBRARY or higher should cause an error here if it's coming from a mod jar.

Copy link
Contributor Author

@shartte shartte Oct 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Early services should mark the discovered jar forcefully as a discovered meaning it should not participate in normal discovery anymore. (Due to being that hacky pre-discovery that is built outside of the locator system).

p.s.: If there isn't a test for this, we should add one.

Copy link
Contributor

@lukebemish lukebemish Oct 5, 2025

Choose a reason for hiding this comment

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

That's not true for IModLanguageLoader, only for the ones in EarlyDiscoveryServices.SERVICES.

}
}
}
if (!issues.isEmpty()) {
throw new ModLoadingException(issues);
}
}

private static ClassProcessor createAccessTransformerService(DiscoveryResult discoveryResult) {
var engine = AccessTransformerEngine.newEngine();
for (var modFile : discoveryResult.gameContent()) {
Expand Down
3 changes: 2 additions & 1 deletion loader/src/main/resources/lang/en_us.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
"fml.modloading.discouragedmod.noreason": "§eNo reason provided§r",
"fml.modloadingissue.dependencyloading.conflictingdependencies": "Some mods have requested conflicting versions of: §6{0}§r. Requested by: §e{1}§r.",
"fml.modloadingissue.dependencyloading.mismatchedcontaineddependencies": "Some mods have agreed upon an acceptable version range for : §6{0}§r, but no jar was provided which matched the range. Requested by: §e{1}§r.",
"fml.modloadingissue.coremod_error": "An error occurred while loading core-mod {0} from {1}",
"fml.modloadingissue.classprocessor_error": "An error occurred while loading class processor {0} from {1}",
"fml.modloadingissue.classprocessor_in_game_content": "Mod file {101} is trying to provide a class processor. Mods and game libraries cannot provide such services, only libraries can.",
"fml.modloadingissue.corrupted_installation": "Your NeoForge installation is corrupted. Please try to reinstall NeoForge.",
"fml.modloadingissue.missing_minecraft_jar": "The patched Minecraft jar is missing. Please try to reinstall NeoForge.",
"fml.modloadingissue.corrupted_minecraft_jar": "The patched Minecraft jar is corrupted. Please try to reinstall NeoForge.",
Expand Down
2 changes: 1 addition & 1 deletion loader/src/test/java/net/neoforged/fml/classloading/JarMetadataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,6 @@ private static ModuleDescriptor getJdkModuleDescriptor(Path testJar) {

@FunctionalInterface
interface ModFileCustomizer {
void customize(ModFileBuilder builder) throws IOException;
void customize(ModFileBuilder<?> builder) throws IOException;
}
}
50 changes: 48 additions & 2 deletions loader/src/test/java/net/neoforged/fml/loading/SimpleProcessorsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.io.IOException;
import java.util.Set;
import net.neoforged.fml.ModLoadingException;
import net.neoforged.fml.testlib.ModFileBuilder;
import net.neoforged.fml.testlib.SimulatedInstallation;
import net.neoforged.fml.testlib.args.ClientInstallationTypesSource;
import net.neoforged.jarjar.metadata.ContainedJarIdentifier;
import net.neoforged.neoforgespi.locating.IModFile;
import net.neoforged.neoforgespi.transformation.ClassProcessor;
Expand All @@ -19,6 +23,7 @@
import net.neoforged.neoforgespi.transformation.SimpleClassProcessor;
import net.neoforged.neoforgespi.transformation.SimpleTransformationContext;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.objectweb.asm.tree.ClassNode;

/**
Expand Down Expand Up @@ -66,7 +71,7 @@ public void createProcessors(Context context, Collector collector) {

var e = assertThrows(ModLoadingException.class, () -> launchAndLoad("neoforgeclient"));
assertThat(getTranslatedIssues(e.getIssues())).containsOnly(
"ERROR: An error occurred while loading core-mod testmod.simpleprocessors.TestSimpleProcessors from mods/testmod.jar > simpleprocessors-1.0.jar");
"ERROR: An error occurred while loading class processor testmod.simpleprocessors.TestSimpleProcessors from mods/testmod.jar > simpleprocessors-1.0.jar");
}

@Test
Expand All @@ -87,7 +92,7 @@ public void createProcessors(Context context, Collector collector) {

var e = assertThrows(ModLoadingException.class, () -> launchAndLoad("neoforgeclient"));
assertThat(getTranslatedIssues(e.getIssues())).containsOnly(
"ERROR: An error occurred while loading core-mod testmod.simpleprocessors.TestSimpleProcessors from mods/simpleprocessors.jar");
"ERROR: An error occurred while loading class processor testmod.simpleprocessors.TestSimpleProcessors from mods/simpleprocessors.jar");
}

@Test
Expand Down Expand Up @@ -119,4 +124,45 @@ public void createProcessors(Context context, Collector collector) {

assertEquals("fml:test", loader.getClassTransformerAuditLog().getAuditString("testmod.TestClass"));
}

/**
* Class processors shouldn't be able to be loaded from mod jars, and the modder should get a proper error
* for this case.
*/
@ParameterizedTest
@ClientInstallationTypesSource
public void testProcessorProvidedByModJar(SimulatedInstallation.Type type) throws Exception {
testProcessorProvidedByGameContent(type, ModFileBuilder::withTestmodModsToml);
}

/**
* Class processors shouldn't be able to be loaded from game libraries, and the modder should get a proper error
* for this case.
*/
@ParameterizedTest
@ClientInstallationTypesSource
public void testProcessorProvidedByGameLibrary(SimulatedInstallation.Type type) throws Exception {
testProcessorProvidedByGameContent(type, builder -> builder.withModTypeManifest("GAMELIBRARY"));
}

private void testProcessorProvidedByGameContent(SimulatedInstallation.Type type, ModFileBuilder.ModJarCustomizer customizer) throws IOException {
installation.setup(type);
installation.buildInstallationAppropriateModProject("testmod", "testmod.jar", builder -> {
customizer.customize(builder);
builder.addClass("testmod.TestProvider", """
public class TestProvider implements net.neoforged.neoforgespi.transformation.ClassProcessorProvider {
@Override
public void createProcessors(Context context, Collector collector) {
throw new RuntimeException();
}
}""")
.addService(ClassProcessorProvider.class, "testmod.TestProvider");
});

var e = assertThrows(ModLoadingException.class, () -> launchAndLoad("neoforgeclient"));
var issues = getTranslatedIssues(e.getIssues());
assertThat(issues).hasSize(1);
assertThat(issues.getFirst()).matches(
"ERROR: Mod file .* is trying to provide a class processor. Mods and game libraries cannot provide such services, only libraries can.");
}
}