Improve feedback when script throws or doesn't send a reply

neoforged · Aug 23, 2023 · f9d14d9 · f9d14d9
1 parent 06a080d
commit f9d14d9
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 15 deletions.
diff --git a/build.gradle b/build.gradle
@@ -80,6 +80,8 @@ dependencies {
     implementation group: 'args4j', name: 'args4j', version: project.arg4j_version
     implementation group: 'com.google.re2j', name: 're2j', version: project.re2j_version
     implementation group: 'it.unimi.dsi', name: 'fastutil', version: project.fastutil_version
+
+    compileOnly 'org.jetbrains:annotations:24.0.1'
 }
 
 jar {

diff --git a/src/main/java/net/neoforged/camelot/commands/utility/EvalCommand.java b/src/main/java/net/neoforged/camelot/commands/utility/EvalCommand.java
@@ -11,11 +11,14 @@
 import net.dv8tion.jda.api.interactions.components.text.TextInputStyle;
 import net.dv8tion.jda.api.interactions.modals.Modal;
 import net.dv8tion.jda.api.interactions.modals.ModalMapping;
+import net.dv8tion.jda.api.requests.RestAction;
 import net.dv8tion.jda.api.utils.messages.MessageCreateBuilder;
+import net.dv8tion.jda.api.utils.messages.MessageCreateData;
 import net.dv8tion.jda.api.utils.messages.MessageEditData;
 import net.neoforged.camelot.BotMain;
 import net.neoforged.camelot.commands.Commands;
 import net.neoforged.camelot.script.ScriptContext;
+import net.neoforged.camelot.script.ScriptReplier;
 import net.neoforged.camelot.script.ScriptUtils;
 
 import java.util.List;
@@ -68,7 +71,12 @@ public static void onEvent(final GenericEvent genericEvent) {
                     .queue(script -> {
                         event.deferReply().queue();
                         final ScriptContext context = new ScriptContext(event.getJDA(), event.getGuild(), event.getMember(),
-                                event.getChannel(), createData -> event.getHook().editOriginal(MessageEditData.fromCreateData(createData)).complete());
+                                event.getChannel(), new ScriptReplier() {
+                            @Override
+                            protected RestAction<?> doSend(MessageCreateData createData) {
+                                return event.getHook().editOriginal(MessageEditData.fromCreateData(createData));
+                            }
+                        });
 
                         ScriptUtils.submitExecution(context, script, Optional.ofNullable(event.getValue("args")).map(ModalMapping::getAsString).orElse(""));
                     });

diff --git a/src/main/java/net/neoforged/camelot/commands/utility/TrickCommand.java b/src/main/java/net/neoforged/camelot/commands/utility/TrickCommand.java
@@ -6,9 +6,12 @@
 import net.dv8tion.jda.api.interactions.commands.OptionMapping;
 import net.dv8tion.jda.api.interactions.commands.OptionType;
 import net.dv8tion.jda.api.interactions.commands.build.OptionData;
+import net.dv8tion.jda.api.requests.RestAction;
+import net.dv8tion.jda.api.utils.messages.MessageCreateData;
 import net.dv8tion.jda.api.utils.messages.MessageEditData;
 import net.neoforged.camelot.db.schemas.Trick;
 import net.neoforged.camelot.script.ScriptContext;
+import net.neoforged.camelot.script.ScriptReplier;
 import net.neoforged.camelot.script.ScriptUtils;
 
 import java.util.List;
@@ -38,7 +41,12 @@ protected void execute(SlashCommandEvent event) {
 
         event.deferReply().queue();
         final ScriptContext context = new ScriptContext(event.getJDA(), event.getGuild(), event.getMember(),
-                event.getChannel(), createData -> event.getHook().editOriginal(MessageEditData.fromCreateData(createData)).complete());
+                event.getChannel(), new ScriptReplier() {
+            @Override
+            protected RestAction<?> doSend(MessageCreateData createData) {
+                return event.getHook().editOriginal(MessageEditData.fromCreateData(createData));
+            }
+        });
 
         ScriptUtils.submitExecution(context, trick.script(), args);
     }

diff --git a/src/main/java/net/neoforged/camelot/listener/TrickListener.java b/src/main/java/net/neoforged/camelot/listener/TrickListener.java
@@ -4,17 +4,18 @@
 import net.dv8tion.jda.api.events.GenericEvent;
 import net.dv8tion.jda.api.events.message.MessageReceivedEvent;
 import net.dv8tion.jda.api.hooks.EventListener;
+import net.dv8tion.jda.api.requests.RestAction;
 import net.dv8tion.jda.api.utils.messages.MessageCreateData;
 import net.dv8tion.jda.api.utils.messages.MessageEditData;
 import net.neoforged.camelot.Database;
 import net.neoforged.camelot.script.ScriptContext;
+import net.neoforged.camelot.script.ScriptReplier;
 import net.neoforged.camelot.script.ScriptUtils;
 import org.jetbrains.annotations.NotNull;
 import net.neoforged.camelot.db.schemas.Trick;
 import net.neoforged.camelot.db.transactionals.TricksDAO;
 
 import java.util.EnumSet;
-import java.util.function.Consumer;
 
 /**
  * A listener listening for {@link MessageReceivedEvent} and seeing if they match a trick alias, which if found,
@@ -39,15 +40,19 @@ public void onEvent(@NotNull GenericEvent gevent) {
 
             final String args = nextSpace < 0 ? "" : content.substring(nextSpace + 1);
 
-            final ScriptContext context = new ScriptContext(event.getJDA(), event.getGuild(), event.getMember(), event.getChannel(), new Consumer<>() {
+            final ScriptContext context = new ScriptContext(event.getJDA(), event.getGuild(), event.getMember(), event.getChannel(), new ScriptReplier() {
                 Message reply;
+
                 @Override
-                public void accept(MessageCreateData create) {
-                    if (reply == null) {
-                        reply = event.getMessage().reply(create)
-                                .setAllowedMentions(ALLOWED_MENTIONS).complete();
-                    } else {
-                        reply.editMessage(MessageEditData.fromCreateData(create)).complete();
+                protected RestAction<?> doSend(MessageCreateData createData) {
+                    synchronized (this) {
+                        if (reply == null) {
+                            return event.getMessage().reply(createData)
+                                    .setAllowedMentions(ALLOWED_MENTIONS)
+                                    .onSuccess(msg -> this.reply = msg);
+                        } else {
+                            return reply.editMessage(MessageEditData.fromCreateData(createData));
+                        }
                     }
                 }
             });

diff --git a/src/main/java/net/neoforged/camelot/script/ScriptContext.java b/src/main/java/net/neoforged/camelot/script/ScriptContext.java
@@ -25,8 +25,7 @@
  * @param reply a consumer used to send a message to the user
  */
 public record ScriptContext(
-        JDA jda, Guild guild, Member member, MessageChannel channel,
-        Consumer<MessageCreateData> reply
+        JDA jda, Guild guild, Member member, MessageChannel channel, ScriptReplier reply
 ) {
     private static final Map<Class<?>, ScriptTransformer<?>> TRANSFORMERS = new IdentityHashMap<>();
 

diff --git a/src/main/java/net/neoforged/camelot/script/ScriptReplier.java b/src/main/java/net/neoforged/camelot/script/ScriptReplier.java
@@ -0,0 +1,45 @@
+package net.neoforged.camelot.script;
+
+import net.dv8tion.jda.api.requests.RestAction;
+import net.dv8tion.jda.api.utils.messages.MessageCreateData;
+import org.jetbrains.annotations.ApiStatus;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
+
+public abstract class ScriptReplier implements Consumer<MessageCreateData> {
+    private final AtomicBoolean wasReplied = new AtomicBoolean();
+
+    /**
+     * Reply to the user and {@link RestAction#complete() immediately complete}
+     * the action.
+     */
+    @Override
+    public void accept(MessageCreateData createData) {
+        send(createData).complete();
+    }
+
+    /**
+     * Reply to the user.
+     */
+    public RestAction<?> send(MessageCreateData createData) {
+        wasReplied.set(true);
+        return doSend(createData);
+    }
+
+    /**
+     * {@return whether the script replied to the user}
+     */
+    public boolean wasReplied() {
+        return wasReplied.get();
+    }
+
+    /**
+     * Reply to the user.
+     *
+     * @deprecated do not use this directly. This is only meant to be overridden. Use {@link #send(MessageCreateData)} instead.
+     */
+    @Deprecated
+    @ApiStatus.OverrideOnly
+    protected abstract RestAction<?> doSend(MessageCreateData createData);
+}
diff --git a/src/main/java/net/neoforged/camelot/script/ScriptUtils.java b/src/main/java/net/neoforged/camelot/script/ScriptUtils.java
@@ -1,7 +1,9 @@
 package net.neoforged.camelot.script;
 
 import net.dv8tion.jda.api.Permission;
+import net.dv8tion.jda.api.entities.Message;
 import net.dv8tion.jda.api.utils.messages.MessageCreateData;
+import net.neoforged.camelot.util.Utils;
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.Engine;
 import org.graalvm.polyglot.EnvironmentAccess;
@@ -150,7 +152,11 @@ public static void submitExecution(ScriptContext context, String script, Object
         final Future<Void> execution = ScriptUtils.SERVICE.submit(() -> ScriptUtils.execute(context, script, args instanceof List<?> ? (List<String>) args : ScriptUtils.toArgs(args.toString())), null);
 
         ScriptUtils.SERVICE.schedule(() -> {
-            if (!execution.isDone()) {
+            if (execution.isDone()) {
+                if (!context.reply().wasReplied()) {
+                    context.reply().send(MessageCreateData.fromContent("Script did not send any reply!")).queue();
+                }
+            } else {
                 execution.cancel(true);
                 context.reply().accept(MessageCreateData.fromContent("Script execution timed out!"));
             }
@@ -214,7 +220,7 @@ public static void execute(
                 } else if (!ex.getMessage().equals("Thread was interrupted.")) { // If it is interrupted then the user would be informed about the time out
                     final StringBuilder message = new StringBuilder();
                     final boolean isCmdLine = ex.getMessage().startsWith(CmdLineParseException.PREFIX);
-                    message.append("Script failed execution due to an exception: **").append(isCmdLine ? ex.getMessage().substring(CmdLineParseException.PREFIX.length()) : ex.getMessage()).append("**");
+                    message.append("Script failed execution due to an exception: **").append(isCmdLine ? ex.getMessage().substring(CmdLineParseException.PREFIX.length()) : Utils.truncate(ex.getMessage(), 1500)).append("**");
                     if (!isCmdLine) {
                         final String trace = String.join("\n", Stream.of(ex.getStackTrace())
                                 .filter(it -> it.getClassName().equals("<js>"))

diff --git a/src/main/java/net/neoforged/camelot/script/SlashTrickManager.java b/src/main/java/net/neoforged/camelot/script/SlashTrickManager.java
@@ -13,7 +13,9 @@
 import net.dv8tion.jda.api.interactions.commands.build.OptionData;
 import net.dv8tion.jda.api.interactions.commands.build.SlashCommandData;
 import net.dv8tion.jda.api.interactions.commands.build.SubcommandData;
+import net.dv8tion.jda.api.requests.RestAction;
 import net.dv8tion.jda.api.requests.restaction.CommandListUpdateAction;
+import net.dv8tion.jda.api.utils.messages.MessageCreateData;
 import net.dv8tion.jda.api.utils.messages.MessageEditData;
 import net.neoforged.camelot.db.schemas.SlashTrick;
 import net.neoforged.camelot.db.transactionals.SlashTricksDAO;
@@ -115,7 +117,12 @@ public void onEvent(@NotNull GenericEvent gevent) {
 
             event.deferReply().queue();
             final ScriptContext context = new ScriptContext(event.getJDA(), event.getGuild(), event.getMember(),
-                    event.getChannel(), createData -> event.getHook().editOriginal(MessageEditData.fromCreateData(createData)).complete());
+                    event.getChannel(), new ScriptReplier() {
+                @Override
+                protected RestAction<?> doSend(MessageCreateData createData) {
+                    return event.getHook().editOriginal(MessageEditData.fromCreateData(createData));
+                }
+            });
 
             ScriptUtils.submitExecution(context, tricksDAO.getTrick(info.id).script(), options);
         }