Skip to content

Commit

Permalink
Add a way for event call sites to handle exceptions gracefully, add a…
Browse files Browse the repository at this point in the history
… whole bunch of extra logging and error handling to RecipesEventJS
  • Loading branch information
MaxNeedsSnacks committed Jul 19, 2023
1 parent f0ebfe5 commit 3586444
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,11 @@
import dev.latvian.mods.kubejs.script.ScriptTypeHolder;
import dev.latvian.mods.kubejs.script.ScriptTypePredicate;
import dev.latvian.mods.kubejs.util.ListJS;
import dev.latvian.mods.rhino.BaseFunction;
import dev.latvian.mods.rhino.Context;
import dev.latvian.mods.rhino.Scriptable;
import dev.latvian.mods.rhino.Wrapper;
import dev.latvian.mods.rhino.*;
import dev.latvian.mods.rhino.util.HideFromJS;
import org.jetbrains.annotations.Nullable;

import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
import java.util.*;
import java.util.function.Consumer;
import java.util.function.Supplier;

Expand Down Expand Up @@ -171,7 +164,7 @@ public void listenJava(ScriptType type, @Nullable Object extraId, IEventHandler
}

/**
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS) ()
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS, EventExceptionHandler)
*/
public EventResult post(EventJS event) {
if (scriptTypePredicate instanceof ScriptTypeHolder type) {
Expand All @@ -182,14 +175,22 @@ public EventResult post(EventJS event) {
}

/**
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS) ()
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS, EventExceptionHandler)
*/
public EventResult post(ScriptTypeHolder scriptType, EventJS event) {
return post(scriptType, null, event);
}

/**
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS) ()
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS, EventExceptionHandler)
*/
// sth, event, exh
public EventResult post(ScriptTypeHolder scriptType, EventJS event, EventExceptionHandler exh) {
return post(scriptType, null, event, exh);
}

/**
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS, EventExceptionHandler)
*/
public EventResult post(EventJS event, @Nullable Object extraId) {
if (scriptTypePredicate instanceof ScriptTypeHolder type) {
Expand All @@ -200,15 +201,33 @@ public EventResult post(EventJS event, @Nullable Object extraId) {
}

/**
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS, EventExceptionHandler)
*/
public EventResult post(EventJS event, @Nullable Object extraId, EventExceptionHandler exh) {
if (scriptTypePredicate instanceof ScriptTypeHolder type) {
return post(type, extraId, event, exh);
} else {
throw new IllegalStateException("You must specify which script type to post event to");
}
}

/**
* @see EventHandler#post(ScriptTypeHolder, Object, EventJS, EventExceptionHandler)
*/
public EventResult post(ScriptTypeHolder type, @Nullable Object extraId, EventJS event) {
return post(type, extraId, event, null);
}

/**
* @return EventResult that can contain an object. What previously returned true on {@link EventJS#cancel()} now returns {@link EventResult#interruptFalse()}
* @see EventJS#cancel()
* @see EventJS#success()
* @see EventJS#exit()
* @see EventJS#cancel(Object)
* @see EventJS#success(Object)
* @see EventJS#exit(Object)
* @return EventResult that can contain an object. What previously returned true on {@link EventJS#cancel()} now returns {@link EventResult#interruptFalse()}
*/
public EventResult post(ScriptTypeHolder type, @Nullable Object extraId, EventJS event) {
public EventResult post(ScriptTypeHolder type, @Nullable Object extraId, EventJS event, EventExceptionHandler exh) {
if (!hasListeners()) {
return EventResult.PASS;
}
Expand All @@ -234,18 +253,18 @@ public EventResult post(ScriptTypeHolder type, @Nullable Object extraId, EventJS
var extraContainers = extraEventContainers == null ? null : extraEventContainers.get(extraId);

if (extraContainers != null) {
postToHandlers(scriptType, extraContainers, event);
postToHandlers(scriptType, extraContainers, event, exh);

if (!scriptType.isStartup()) {
postToHandlers(ScriptType.STARTUP, extraContainers, event);
postToHandlers(ScriptType.STARTUP, extraContainers, event, exh);
}
}

if (eventContainers != null) {
postToHandlers(scriptType, eventContainers, event);
postToHandlers(scriptType, eventContainers, event, exh);

if (!scriptType.isStartup()) {
postToHandlers(ScriptType.STARTUP, eventContainers, event);
postToHandlers(ScriptType.STARTUP, eventContainers, event, exh);
}
}
} catch (EventExit exit) {
Expand All @@ -268,11 +287,11 @@ public EventResult post(ScriptTypeHolder type, @Nullable Object extraId, EventJS
return eventResult;
}

private void postToHandlers(ScriptType type, EventHandlerContainer[] containers, EventJS event) throws EventExit {
private void postToHandlers(ScriptType type, EventHandlerContainer[] containers, EventJS event, @Nullable EventExceptionHandler exh) throws EventExit {
var handler = containers[type.ordinal()];

if (handler != null) {
handler.handle(event);
handler.handle(event, exh);
}
}

Expand Down Expand Up @@ -339,4 +358,19 @@ public Set<Object> findUniqueExtraIds(ScriptType type) {

return set;
}

@FunctionalInterface
public interface EventExceptionHandler {
/**
* Handles an exception thrown by an event handler.
*
* @param event The event being posted
* @param container The event handler container that threw the exception
* @param ex The exception that was thrown
* @return <code>null</code> if the exception could be recovered from, otherwise the exception that should be rethrown
* @implNote The thrown exception will never be an instance of {@link EventExit} or {@link WrappedException},
* as those are already handled by the container itself.
*/
Throwable handle(EventJS event, EventHandlerContainer container, Throwable ex);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package dev.latvian.mods.kubejs.event;

import dev.latvian.mods.rhino.Undefined;
import dev.latvian.mods.rhino.WrappedException;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -32,7 +31,7 @@ public EventHandlerContainer(Object extraId, IEventHandler handler, String sourc
this.line = line;
}

public EventResult handle(EventJS event) throws EventExit {
public EventResult handle(EventJS event, EventHandler.EventExceptionHandler exh) throws EventExit {
var itr = this;

do {
Expand All @@ -47,7 +46,13 @@ public EventResult handle(EventJS event) throws EventExit {
throwable = e.getWrappedException();
}

throw throwable instanceof EventExit exit ? exit : EventResult.Type.ERROR.exit(throwable);
if (throwable instanceof EventExit exit) {
throw exit;
}

if (exh == null || (throwable = exh.handle(event, itr, throwable)) != null) {
throw EventResult.Type.ERROR.exit(throwable);
}
}

itr = itr.child;
Expand All @@ -66,4 +71,9 @@ public void add(Object extraId, IEventHandler handler, String source, int line)

itr.child = new EventHandlerContainer(extraId, handler, source, line);
}

@Override
public String toString() {
return "Event Handler (" + source + ":" + line + ")";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,16 @@ public RecipeTypeFunction(RecipesEventJS event, RecipeSchemaType schemaType) {

@Override
public RecipeJS call(Context cx, Scriptable scope, Scriptable thisObj, Object[] args0) {
return createRecipe(args0);
try {
return createRecipe(args0);
} catch (RecipeExceptionJS rex) {
if (rex.error) {
throw rex;
} else {
ConsoleJS.SERVER.error("Failed to create recipe for type '" + id + "'", rex, SKIP_ERROR);
return null;
}
}
}

public RecipeJS createRecipe(Object[] args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import dev.latvian.mods.kubejs.DevProperties;
import dev.latvian.mods.kubejs.bindings.event.ServerEvents;
import dev.latvian.mods.kubejs.core.RecipeKJS;
import dev.latvian.mods.kubejs.event.EventHandler;
import dev.latvian.mods.kubejs.event.EventJS;
import dev.latvian.mods.kubejs.item.ingredient.IngredientWithCustomPredicate;
import dev.latvian.mods.kubejs.item.ingredient.TagContext;
Expand Down Expand Up @@ -44,22 +45,8 @@
import org.jetbrains.annotations.Nullable;

import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.ForkJoinTask;
import java.util.concurrent.ForkJoinWorkerThread;
import java.util.*;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;
import java.util.function.Function;
Expand All @@ -71,6 +58,15 @@
public class RecipesEventJS extends EventJS {
public static final Pattern SKIP_ERROR = Pattern.compile("at.*dev\\.latvian\\.mods\\.kubejs\\.recipe\\.RecipesEventJS\\.post");
private static final Predicate<RecipeJS> RECIPE_NOT_REMOVED = r -> r != null && !r.removed;
private static final EventHandler.EventExceptionHandler RECIPE_EXCEPTION_HANDLER = (event, handler, ex) -> {
// skip the current handler on a recipe or JSON exception, but let other handlers run
if (ex instanceof RecipeExceptionJS || ex instanceof JsonParseException) {
ConsoleJS.SERVER.error("Error while processing recipe event handler: " + handler, ex);
return null;
} else {
return ex; // rethrow
}
};

public static final Map<UUID, ModifyRecipeResultCallback> MODIFY_RESULT_CALLBACKS = new HashMap<>();

Expand All @@ -95,7 +91,13 @@ public class RecipesEventJS extends EventJS {
System.exit(-1);
}

ConsoleJS.SERVER.error(String.format("Caught exception in thread %s while performing async operation!", thread), ex);
ConsoleJS.SERVER.error("Error in thread %s while performing bulk recipe operation!".formatted(thread), ex);

RecipeExceptionJS rex = ex instanceof RecipeExceptionJS rex1 ? rex1 : new RecipeExceptionJS(null, ex).error();

if (rex.error) {
throw rex;
}
}, true);

public static Map<UUID, IngredientWithCustomPredicate> customIngredientMap = null;
Expand Down Expand Up @@ -527,19 +529,28 @@ public RecipeTypeFunction getRecipeFunction(@Nullable String id) {
}

public RecipeJS custom(JsonObject json) {
if (json == null || !json.has("type")) {
throw new RecipeExceptionJS("JSON must contain 'type'!");
}
try {
if (json == null || !json.has("type")) {
throw new RecipeExceptionJS("JSON must contain 'type'!");
}

var type = getRecipeFunction(json.get("type").getAsString());
var type = getRecipeFunction(json.get("type").getAsString());

if (type == null) {
throw new RecipeExceptionJS("Unknown recipe type: " + json.get("type").getAsString());
}
if (type == null) {
throw new RecipeExceptionJS("Unknown recipe type: " + json.get("type").getAsString());
}

var recipe = type.schemaType.schema.deserialize(type, null, json);
recipe.afterLoaded();
return addRecipe(recipe, true);
var recipe = type.schemaType.schema.deserialize(type, null, json);
recipe.afterLoaded();
return addRecipe(recipe, true);
} catch (RecipeExceptionJS rex) {
if (rex.error) {
throw rex;
} else {
ConsoleJS.SERVER.error("Failed to create custom JSON recipe from '%s'".formatted(json), rex, SKIP_ERROR);
return null;
}
}
}

private void printTypes(Predicate<RecipeSchemaType> predicate) {
Expand Down

0 comments on commit 3586444

Please sign in to comment.