Skip to content

Commit

Permalink
Fix exceptions directly thrown by init tasks not being reported as mo…
Browse files Browse the repository at this point in the history
…d loading issues. (#142)
  • Loading branch information
shartte authored May 20, 2024
1 parent 99d3cc4 commit 533713b
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 8 deletions.
45 changes: 37 additions & 8 deletions loader/src/main/java/net/neoforged/fml/ModLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -259,24 +259,53 @@ private static void waitForFuture(String name, Runnable periodicTask, Completabl
return;
} catch (ExecutionException e) {
// Merge all potential modloading issues
var errorCount = 0;
var issueCountBefore = loadingIssues.size();
// Add the cause itself if it seems meaningful based on its type and/or message, since we'd
// present this exception to the user using toString().
// "RuntimeException: null" or "IllegalStateException: null" isn't meaningful.
if (isMeaningfulException(e.getCause())) {
addLoadingIssuesFromException(name, e.getCause());
}
for (var error : e.getCause().getSuppressed()) {
if (error instanceof DependentFutureFailedException) {
continue;
} else if (error instanceof ModLoadingException modLoadingException) {
loadingIssues.addAll(modLoadingException.getIssues());
} else {
loadingIssues.add(ModLoadingIssue.error("fml.modloading.uncaughterror", name).withCause(e));
// When a prerequisite of the task failed due to an exception, skip this so only
// the root cause is reported.
if (!(error instanceof DependentFutureFailedException)) {
addLoadingIssuesFromException(name, error);
}
errorCount++;
}
// If we discarded all exceptions, we'd report an empty issue list while still canceling the loading process
// Fall back to using the cause.
if (loadingIssues.isEmpty()) {
addLoadingIssuesFromException(name, e.getCause());
}

var errorCount = loadingIssues.size() - issueCountBefore;
LOGGER.fatal(LOADING, "Failed to wait for future {}, {} errors found", name, errorCount);
cancelLoading(modList);
throw new ModLoadingException(loadingIssues);
} catch (Exception ignored) {}
}
}

private static boolean isMeaningfulException(Throwable error) {
var message = error.getLocalizedMessage();
if (message != null && !message.isBlank()) {
return true; // We'd consider any exception with a message as meaningful
}
// The trifecta of generic exceptions...
return !error.getClass().isAssignableFrom(RuntimeException.class)
&& !error.getClass().isAssignableFrom(IllegalStateException.class)
&& !error.getClass().isAssignableFrom(IllegalArgumentException.class);
}

private static void addLoadingIssuesFromException(String context, Throwable error) {
if (error instanceof ModLoadingException modLoadingException) {
loadingIssues.addAll(modLoadingException.getIssues());
} else {
loadingIssues.add(ModLoadingIssue.error("fml.modloading.uncaughterror", context).withCause(error));
}
}

private static List<ModContainer> buildMods(final IModFile modFile) {
final Map<IModLanguageLoader, Set<ModContainer>> byLoader = new IdentityHashMap<>();
var containers = modFile.getModFileInfo()
Expand Down
43 changes: 43 additions & 0 deletions loader/src/test/java/net/neoforged/fml/loading/FMLLoaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
import java.nio.file.Files;
import java.util.List;
import java.util.Map;
import net.neoforged.fml.ModLoader;
import net.neoforged.fml.ModLoadingException;
import net.neoforged.fml.ModLoadingIssue;
import net.neoforged.fml.ModWorkManager;
import net.neoforged.fml.event.lifecycle.FMLClientSetupEvent;
import net.neoforged.jarjar.metadata.ContainedJarIdentifier;
import net.neoforged.jarjar.metadata.ContainedJarMetadata;
import net.neoforged.jarjar.metadata.ContainedVersion;
Expand Down Expand Up @@ -450,6 +453,46 @@ void testMissingOrUnsatisfiedForgeFeatures() throws Exception {
"ERROR: testmod (testmod) is missing a feature it requires to run"
+ "\nIt requires thisFeatureDoesNotExist=\"*\" but NONE is available");
}

@Test
void testExceptionInParallelEventDispatchIsCollectedAsModLoadingIssue() throws Exception {
installation.setupProductionClient();

installation.buildModJar("testmod.jar")
.withTestmodModsToml()
.addClass("testmod.Thrower", """
@net.neoforged.fml.common.Mod("testmod")
public class Thrower {
public Thrower(net.neoforged.bus.api.IEventBus modEventBus) {
modEventBus.addListener(net.neoforged.fml.event.lifecycle.FMLClientSetupEvent.class, e -> {
throw new IllegalStateException("Exception Message");
});
}
}
""")
.build();

var launchResult = launch("forgeclient");
assertThat(launchResult.loadedMods()).containsKey("testmod");
loadMods(launchResult);
var e = assertThrows(ModLoadingException.class, () -> ModLoader.dispatchParallelEvent("test", ModWorkManager.syncExecutor(), ModWorkManager.parallelExecutor(), () -> {}, FMLClientSetupEvent::new));
assertThat(getTranslatedIssues(e.getIssues())).containsOnly(
"ERROR: testmod (testmod) encountered an error while dispatching the net.neoforged.fml.event.lifecycle.FMLClientSetupEvent event\n"
+ "java.lang.IllegalStateException: Exception Message");
}

@Test
void testExceptionInInitTaskIsCollectedAsModLoadingIssue() throws Exception {
installation.setupProductionClient();

loadMods(launch("forgeclient"));
var e = assertThrows(ModLoadingException.class, () -> ModLoader.runInitTask("test", ModWorkManager.syncExecutor(), () -> {}, () -> {
throw new IllegalStateException("Exception Message");
}));
assertThat(getTranslatedIssues(e.getIssues())).containsOnly(
"ERROR: An uncaught parallel processing error has occurred."
+ "\njava.lang.IllegalStateException: Exception Message");
}
}

@Nested
Expand Down
12 changes: 12 additions & 0 deletions loader/src/test/java/net/neoforged/fml/loading/ModFileBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ public ModFileBuilder(Path destination) throws IOException {
compilationBuilder = compiler.builder();
}

public ModFileBuilder withTestmodModsToml() {
return withTestmodModsToml(ignored -> {});
}

public ModFileBuilder withTestmodModsToml(Consumer<ModsTomlBuilder> customizer) {
return withModsToml(builder -> {
builder.unlicensedJavaMod();
builder.addMod("testmod", "1.0");
customizer.accept(builder);
});
}

public ModFileBuilder withModsToml(Consumer<ModsTomlBuilder> customizer) {
var modsToml = new ModsTomlBuilder();
customizer.accept(modsToml);
Expand Down

0 comments on commit 533713b

Please sign in to comment.