[Bug] Cloud Doesn't Throw Exceptions for Brigadier Registered Commands

[Axionize]

Cloud's exception handlers do not handle/process misspelt brigadier commands and throw an InvalidSyntaxException as you would expect.

Packet logging and debugging confirms the commands are being sent by the client. This bug exists in both Bukkit/Spigot/Paper and Modded, I will be using Fabric and its associated mappings below because it was easier to debug to explain this issue in death.

For commands that exist (not misspelt) they are executed here in ContextChain.java

    public static <S> int runExecutable(final CommandContext<S> executable, final S source, final ResultConsumer<S> resultConsumer, final boolean forkedMode) throws CommandSyntaxException {
        final CommandContext<S> contextToUse = executable.copyFor(source);
        try {
            final int result = executable.getCommand().run(contextToUse); // Cloud is run by this line over here

            resultConsumer.onCommandComplete(contextToUse, true, result);
            return forkedMode ? 1 : result;
        } catch (final CommandSyntaxException ex) {
            resultConsumer.onCommandComplete(contextToUse, false, 0);
            if (forkedMode) {
                return 0;
            }
            throw ex;
        }
    }

Which calls and executes https://github.com/Incendo/cloud-minecraft/blob/master/cloud-brigadier/src/main/java/org/incendo/cloud/brigadier/CloudBrigadierCommand.java#L94 with .run()

The stack trace for processing the command server side looks something like this

handleHelp:24, GrimHelp (ac.grim.grimac.command.commands)
execute:-1, GrimHelp$$Lambda/0x00007e230cdb8900 (ac.grim.grimac.command.commands)
executeFuture:91, CommandExecutionHandler (org.incendo.cloud.execution)
lambda$coordinateExecution$4:121, ExecutionCoordinatorImpl (org.incendo.cloud.execution)
apply:-1, ExecutionCoordinatorImpl$$Lambda/0x00007e230d8f97c8 (org.incendo.cloud.execution)
tryFire:1150, CompletableFuture$UniCompose (java.util.concurrent)
run:482, CompletableFuture$Completion (java.util.concurrent)
execute:53, ExecutionCoordinatorImpl$NonSchedulingExecutor (org.incendo.cloud.execution)
uniComposeStage:1184, CompletableFuture (java.util.concurrent)
thenComposeAsync:2352, CompletableFuture (java.util.concurrent)
coordinateExecution:104, ExecutionCoordinatorImpl (org.incendo.cloud.execution)
executeCommand:91, StandardCommandExecutor (org.incendo.cloud)
executeCommand:65, StandardCommandExecutor (org.incendo.cloud)
run:94, CloudBrigadierCommand (org.incendo.cloud.brigadier) <--- **Cloud entrypoint**
runExecutable:73, ContextChain (com.mojang.brigadier.context)
execute:26, FixedCommandAction (net.minecraft.command)
execute:-1, FixedCommandAction (net.minecraft.command)
method_54405:8, SourcedCommandAction (net.minecraft.command)
execute:-1, SourcedCommandAction$$Lambda/0x00007e230d8f2360 (net.minecraft.command)
execute:5, CommandQueueEntry (net.minecraft.command)
run:101, CommandExecutionContext (net.minecraft.command)
callWithContext:294, CommandManager (net.minecraft.server.command)
execute:223, CommandManager (net.minecraft.server.command) <----**Here is where successful execution vs misspelt commands differ**
executeCommand:1307, ServerPlayNetworkHandler (net.minecraft.server.network)
method_44356:1296, ServerPlayNetworkHandler (net.minecraft.server.network)
run:-1, ServerPlayNetworkHandler$$Lambda/0x00007e230d8eb988 (net.minecraft.server.network)
run:17, ServerTask (net.minecraft.server)
executeTask:144, ThreadExecutor (net.minecraft.util.thread)
executeTask:24, ReentrantThreadExecutor (net.minecraft.util.thread)
executeTask:935, MinecraftServer (net.minecraft.server)
executeTask:-1, MinecraftServer (net.minecraft.server)
runTask:118, ThreadExecutor (net.minecraft.util.thread)
runOneTask:918, MinecraftServer (net.minecraft.server)
runTask:912, MinecraftServer (net.minecraft.server)
runTasks:128, ThreadExecutor (net.minecraft.util.thread)
runTasks:877, MinecraftServer (net.minecraft.server)
runTasksTillTickEnd:885, MinecraftServer (net.minecraft.server)
runServer:769, MinecraftServer (net.minecraft.server)
method_29739:299, MinecraftServer (net.minecraft.server)
run:-1, MinecraftServer$$Lambda/0x00007e230d2a1e18 (net.minecraft.server)
runWith:1596, Thread (java.lang)
run:1583, Thread (java.lang)

The difference is here:

* Executes {@code command}. The command cannot be prefixed with a slash.
     * 
     * @see #executeWithPrefix(ServerCommandSource, String)
     */
    public void execute(ParseResults<ServerCommandSource> parseResults, String command) {
        ServerCommandSource serverCommandSource = parseResults.getContext().getSource();
        Profilers.get().push((Supplier<String>)(() -> "/" + command));
        ContextChain<ServerCommandSource> contextChain = checkCommand(parseResults, command, serverCommandSource); 

        try { // **for a command that exists contextChain is not null, if no command exists it returns null**
            if (contextChain != null) { 
                callWithContext(
                    serverCommandSource, context -> CommandExecutionContext.enqueueCommand(context, command, contextChain, serverCommandSource, ReturnValueConsumer.EMPTY) // **this eventually leads to /grim help execution**
                );
            }
        } catch (Exception var12) {
            MutableText mutableText = Text.literal(var12.getMessage() == null ? var12.getClass().getName() : var12.getMessage());
            if (LOGGER.isDebugEnabled()) {
                LOGGER.error("Command exception: /{}", command, var12);
                StackTraceElement[] stackTraceElements = var12.getStackTrace();

                for (int i = 0; i < Math.min(stackTraceElements.length, [3](https://mclo.gs/TYuWdgE#L3)); i++) {
                    mutableText.append("\n\n")
                        .append(stackTraceElements[i].getMethodName())
                        .append("\n ")
                        .append(stackTraceElements[i].getFileName())
                        .append(":")
                        .append(String.valueOf(stackTraceElements[i].getLineNumber()));
                }
            }

            serverCommandSource.sendError(Text.translatable("command.failed").styled(style -> style.withHoverEvent(new HoverEvent(Action.SHOW_TEXT, mutableText))));
            if (SharedConstants.isDevelopment) {
                serverCommandSource.sendError(Text.literal(Util.getInnermostMessage(var[12](https://mclo.gs/TYuWdgE#L12))));
                LOGGER.error("'/{}' threw an exception", command, var12);
            }
        } finally {
            Profilers.get().pop();
        }
    }

Ordinarily I'd make a PR and fix this on my own but I'm not familiar enough with Cloud to even begin to know how to fix this nicely in a way that would get upstreamed and I would appreciate some help in this matter.

1. How does cloud-brigadier register itself to handler these commands exactly?
2. How should we intercept execution of these non-existence/failed commands so that clouds exception handlers can take over them?

On modded platforms a simple Mixin will do to grab the parsed input when the contextChain is null and then just run it through cloud as a command but I'm not sure sure on Bukkit or other platforms, and I'm not even sure if that's ideal. Is there another cloud method or class we should be looking at to just directly parse only for exceptions or raise the exception with the input or will that break other things? Should we find a different place to try and hook into to fix this behaviour?

[Axionize]

@jpenilla @Citymonstret @Konicai It looks like you're the authors of or contributors too the relevant code. I'd appreciate any insight you can give me what you all think is the best way to solve this.