diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index 869d7ccb7..259a1ff24 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -182,6 +182,7 @@ final class SuggestionsProvider { final LiteralCommandNode alias, final StringReader reader, final CommandContextBuilder contextSoFar) { final S source = contextSoFar.getSource(); + final String fullInput = reader.getString(); final VelocityArgumentCommandNode argsNode = VelocityCommands.getArgumentsNode(alias); if (argsNode == null) { // This is a BrigadierCommand, fallback to regular suggestions @@ -212,15 +213,13 @@ final class SuggestionsProvider { this.getArgumentsNodeSuggestions(argsNode, reader, context); final boolean hasHints = alias.getChildren().size() > 1; if (!hasHints) { - return cmdSuggestions; + return this.merge(fullInput, cmdSuggestions); } // Parse the hint nodes to get remaining suggestions reader.setCursor(start); final CompletableFuture hintSuggestions = this.getHintSuggestions(alias, reader, contextSoFar); - - final String fullInput = reader.getString(); return this.merge(fullInput, cmdSuggestions, hintSuggestions); } @@ -245,7 +244,7 @@ final class SuggestionsProvider { return node.listSuggestions(built, new SuggestionsBuilder(fullInput, start)); } catch (final Throwable e) { // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. - LOGGER.error("Arguments node cannot provide suggestions, skipping", e); + LOGGER.error("Arguments node cannot provide suggestions", e); return Suggestions.empty(); } } @@ -264,7 +263,13 @@ final class SuggestionsProvider { final LiteralCommandNode alias, final StringReader reader, final CommandContextBuilder context) { final ParseResults parse = this.parseHints(alias, reader, context); - return this.dispatcher.getCompletionSuggestions(parse); + try { + return this.dispatcher.getCompletionSuggestions(parse); + } catch (final Throwable e) { + // Again, plugins are naughty. + LOGGER.error("Hint node cannot provide suggestions", e); + return Suggestions.empty(); + } } /** diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index 3c721fc95..66fed8234 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -23,10 +23,13 @@ import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; +import com.velocitypowered.api.command.SimpleCommand; import java.util.List; +import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.Test; /** @@ -65,6 +68,51 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("bar"); } + @Test + void testDoesNotSuggestForPartialIncorrectAlias() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("yo"); + assertSuggestions("welcome"); + } + + @Test + void testDoesNotSuggestArgumentsForPartialAlias() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + + assertSuggestions("hell "); + } + + @Test + void testSuggestsArgumentsIgnoresAliasCase() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("Hello ", "world"); + } + @Test void testSuggestsHintLiteral() { final var hint = LiteralArgumentBuilder @@ -123,6 +171,102 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("foo bar", "baz", "qux"); } + // Exception handling + + @Test + void testSkipsSuggestionWhenNodeSuggestionProviderFutureCompletesExceptionally() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + } + }); + + assertSuggestions("hello "); + } + + @Test + void testSkipsSuggestionWhenNodeSuggestionProviderThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + throw new RuntimeException(); + } + }); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSkipsSuggestionWhenHintSuggestionProviderFutureCompletesExceptionally() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("hello "); + } + + @Test + void testSkipsSuggestionWhenHintSuggestionProviderThrows() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> { + throw new RuntimeException(); + }) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSuggestionMergingIgnoresExceptionallyCompletedSuggestionFutures() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("hello ", "world"); + } + static final class NoSuggestionsCommand implements RawCommand { static final NoSuggestionsCommand INSTANCE = new NoSuggestionsCommand();