From 9430fcc0529e68a3e9847d2d121e47a7c3b1695a Mon Sep 17 00:00:00 2001 From: gnembon Date: Mon, 27 Jan 2020 22:47:29 -0400 Subject: [PATCH] handled better errors and stack traces for more (all) cases --- .../java/carpet/commands/ScriptCommand.java | 19 +++--- .../java/carpet/script/CarpetScriptHost.java | 65 ++++++++++++++++--- .../carpet/script/CarpetScriptServer.java | 6 +- src/main/java/carpet/script/ScriptHost.java | 7 ++ .../carpet/script/value/FunctionValue.java | 2 +- .../java/carpet/script/value/ThreadValue.java | 14 +++- 6 files changed, 88 insertions(+), 25 deletions(-) diff --git a/src/main/java/carpet/commands/ScriptCommand.java b/src/main/java/carpet/commands/ScriptCommand.java index 5ce20423..ac594cd9 100644 --- a/src/main/java/carpet/commands/ScriptCommand.java +++ b/src/main/java/carpet/commands/ScriptCommand.java @@ -276,16 +276,19 @@ public class ScriptCommand } private static CarpetScriptHost getHost(CommandContext context) { + CarpetScriptHost host; try { String name = StringArgumentType.getString(context, "app").toLowerCase(Locale.ROOT); CarpetScriptHost parentHost = CarpetServer.scriptServer.modules.getOrDefault(name, CarpetServer.scriptServer.globalHost); - return parentHost.retrieveForExecution(context.getSource()); + host = parentHost.retrieveForExecution(context.getSource()); } catch (IllegalArgumentException ignored) { - return CarpetServer.scriptServer.globalHost; + host = CarpetServer.scriptServer.globalHost; } + host.setChatErrorSnooper(context.getSource()); + return host; } private static Collection suggestFunctionCalls(CommandContext c) { @@ -324,7 +327,6 @@ public class ScriptCommand Messenger.m(source, "gi ----------------"); return; } - CarpetSettings.LOG.error("printing function "+fun.getPrettyString()); Expression expr = fun.getExpression(); Tokenizer.Token tok = fun.getToken(); List snippet = ExpressionInspector.Expression_getExpressionSnippet(tok, expr); @@ -375,10 +377,9 @@ public class ScriptCommand } catch (CarpetExpressionException e) { - e.printStack(source); - Messenger.m(source, "r Exception while evaluating expression at "+new BlockPos(source.getPosition())+": "+e.getMessage()); + host.handleErrorWithStack("Error while evaluating expression", e); } - host.resetErrorSnooper(); + //host.resetErrorSnooper(); // lets say no need to reset the snooper in case something happens on the way } private static int invoke(CommandContext context, String call, BlockPos pos1, BlockPos pos2, String args) @@ -457,8 +458,7 @@ public class ScriptCommand } catch (CarpetExpressionException exc) { - exc.printStack(source); - Messenger.m(source, "r Error while processing command: "+exc); + host.handleErrorWithStack("Error while processing command", exc); return 0; } finally @@ -505,8 +505,7 @@ public class ScriptCommand } catch (CarpetExpressionException e) { - e.printStack(source); - Messenger.m(source, "r Exception while filling the area:\n","l "+e.getMessage()); + host.handleErrorWithStack("Exception while filling the area", e); return 0; } catch (ArithmeticException e) diff --git a/src/main/java/carpet/script/CarpetScriptHost.java b/src/main/java/carpet/script/CarpetScriptHost.java index fb307910..354dba7d 100644 --- a/src/main/java/carpet/script/CarpetScriptHost.java +++ b/src/main/java/carpet/script/CarpetScriptHost.java @@ -32,6 +32,7 @@ import static java.lang.Math.max; public class CarpetScriptHost extends ScriptHost { private final CarpetScriptServer scriptServer; + ServerCommandSource responsibleSource; private Tag globalState; private int saveTimeout; @@ -66,8 +67,7 @@ public class CarpetScriptHost extends ScriptHost } catch (CarpetExpressionException e) { - e.printStack(source); - Messenger.m(source, "r Exception while evaluating expression at " + new BlockPos(source.getPosition()) + ": " + e.getMessage()); + host.handleErrorWithStack("Error while evaluating expression", e); host.resetErrorSnooper(); return null; } @@ -129,16 +129,33 @@ public class CarpetScriptHost extends ScriptHost public CarpetScriptHost retrieveForExecution(ServerCommandSource source) { - if (!perUser) - return this; + CarpetScriptHost host = this; + if (perUser) + { + try + { + ServerPlayerEntity player = source.getPlayer(); + host = (CarpetScriptHost) retrieveForExecution(player.getName().getString()); + } + catch (CommandSyntaxException e) + { + host = (CarpetScriptHost) retrieveForExecution((String) null); + } + } + if (host.errorSnooper == null) host.setChatErrorSnooper(source); + return host; + } + + public String handleCommand(ServerCommandSource source, String call, List coords, String arg) + { try { - ServerPlayerEntity player = source.getPlayer(); - return (CarpetScriptHost) retrieveForExecution(player.getName().getString()); + return call(source, call, coords, arg); } - catch (CommandSyntaxException e) + catch (CarpetExpressionException exc) { - return (CarpetScriptHost) retrieveForExecution((String)null); + handleErrorWithStack("Error while running custom command", exc); + return ""; } } @@ -236,7 +253,7 @@ public class CarpetScriptHost extends ScriptHost } catch (ExpressionException e) { - return e.getMessage(); + throw new CarpetExpressionException(e.getMessage(), e.stack); } } @@ -259,7 +276,7 @@ public class CarpetScriptHost extends ScriptHost } catch (ExpressionException e) { - CarpetSettings.LOG.error("Callback failed: "+e.getMessage()); + handleExpressionException("Callback failed", e); } return Value.NULL; } @@ -361,6 +378,7 @@ public class CarpetScriptHost extends ScriptHost public void setChatErrorSnooper(ServerCommandSource source) { + responsibleSource = source; errorSnooper = (expr, /*Nullable*/ token, message) -> { try @@ -412,4 +430,31 @@ public class CarpetScriptHost extends ScriptHost return new ArrayList<>(); }; } + + @Override + public void resetErrorSnooper() + { + responsibleSource = null; + super.resetErrorSnooper(); + } + + public void handleErrorWithStack(String intro, CarpetExpressionException exception) + { + if (responsibleSource != null) + { + exception.printStack(responsibleSource); + String message = exception.getMessage(); + Messenger.m(responsibleSource, "r "+intro+(message.isEmpty()?"":": "+message)); + } + else + { + CarpetSettings.LOG.error(intro+": "+exception.getMessage()); + } + } + + @Override + public void handleExpressionException(String message, ExpressionException exc) + { + handleErrorWithStack(message, new CarpetExpressionException(exc.getMessage(), exc.stack)); + } } diff --git a/src/main/java/carpet/script/CarpetScriptServer.java b/src/main/java/carpet/script/CarpetScriptServer.java index 62d0c347..02761df8 100644 --- a/src/main/java/carpet/script/CarpetScriptServer.java +++ b/src/main/java/carpet/script/CarpetScriptServer.java @@ -220,7 +220,7 @@ public class CarpetScriptServer executes( (c) -> { String response = modules.get(hostName).retrieveForExecution(c.getSource()). - call(c.getSource(),"__command", null, ""); + handleCommand(c.getSource(),"__command", null, ""); if (!response.isEmpty()) Messenger.m(c.getSource(), "gi "+response); return 1; }); @@ -232,14 +232,14 @@ public class CarpetScriptServer requires((player) -> modules.containsKey(hostName) && modules.get(hostName).getFunction(function) != null). executes( (c) -> { String response = modules.get(hostName).retrieveForExecution(c.getSource()). - call(c.getSource(), function,null,""); + handleCommand(c.getSource(), function,null,""); if (!response.isEmpty()) Messenger.m(c.getSource(),"gi "+response); return 1; }). then(argument("args...", StringArgumentType.greedyString()). executes( (c) -> { String response = modules.get(hostName).retrieveForExecution(c.getSource()). - call(c.getSource(), function,null, StringArgumentType.getString(c, "args...")); + handleCommand(c.getSource(), function,null, StringArgumentType.getString(c, "args...")); if (!response.isEmpty()) Messenger.m(c.getSource(), "gi "+response); return 1; }))); diff --git a/src/main/java/carpet/script/ScriptHost.java b/src/main/java/carpet/script/ScriptHost.java index 63ef1dc2..37f0de3f 100644 --- a/src/main/java/carpet/script/ScriptHost.java +++ b/src/main/java/carpet/script/ScriptHost.java @@ -1,6 +1,7 @@ package carpet.script; import carpet.script.bundled.Module; +import carpet.script.exception.ExpressionException; import carpet.script.exception.InternalExpressionException; import carpet.script.value.FunctionValue; import carpet.script.value.Value; @@ -313,6 +314,12 @@ public abstract class ScriptHost return userHost; } + public void handleExpressionException(String msg, ExpressionException exc) + { + System.out.println(msg+": "+exc); + } + + protected abstract ScriptHost duplicate(); public void onClose() { } diff --git a/src/main/java/carpet/script/value/FunctionValue.java b/src/main/java/carpet/script/value/FunctionValue.java index aeab0d73..14f6498d 100644 --- a/src/main/java/carpet/script/value/FunctionValue.java +++ b/src/main/java/carpet/script/value/FunctionValue.java @@ -68,7 +68,7 @@ public class FunctionValue extends Value implements Fluff.ILazyFunction return (name.equals("_")?"":name) +"("+String.join(", ",stringArgs)+")"; } - public String fullName() {return expression.module == null?name:name+"["+expression.module.getName()+"]";} + public String fullName() {return (name.equals("_")?"":name)+(expression.module == null?"":"["+expression.module.getName()+"]");} @Override public boolean getBoolean() diff --git a/src/main/java/carpet/script/value/ThreadValue.java b/src/main/java/carpet/script/value/ThreadValue.java index 6d3998f6..98ac7623 100644 --- a/src/main/java/carpet/script/value/ThreadValue.java +++ b/src/main/java/carpet/script/value/ThreadValue.java @@ -4,6 +4,7 @@ import carpet.script.Context; import carpet.script.Expression; import carpet.script.LazyValue; import carpet.script.Tokenizer; +import carpet.script.exception.ExpressionException; import carpet.script.exception.InternalExpressionException; import java.util.HashMap; @@ -29,7 +30,18 @@ public class ThreadValue extends Value { id = sequence++; taskFuture = CompletableFuture.supplyAsync( - () -> function.lazyEval(ctx, Context.NONE, expr, token, args).evalValue(ctx), + () -> + { + try + { + return function.lazyEval(ctx, Context.NONE, expr, token, args).evalValue(ctx); + } + catch (ExpressionException exc) + { + ctx.host.handleExpressionException("Thread failed", exc); + return Value.NULL; + } + }, executorServices.computeIfAbsent(pool, (v) -> (ThreadPoolExecutor)Executors.newCachedThreadPool()) ); Thread.yield();