From 7edd53ccf3bf26059cd2513b49c5f7ad50f75826 Mon Sep 17 00:00:00 2001 From: Alathreon Date: Thu, 10 Aug 2023 15:28:16 +0200 Subject: [PATCH] JShell now allows wrong uri and dead server, will give correct response to the user and will log them as warns --- .../tjbot/config/JShellConfig.java | 11 +--- .../tjbot/features/code/EvalCodeCommand.java | 3 +- .../tjbot/features/jshell/JShellCommand.java | 13 +++- .../tjbot/features/jshell/JShellEval.java | 3 +- .../features/jshell/backend/JShellApi.java | 59 +++++++++++++++---- .../utils/ConnectionFailedException.java | 34 +++++++++++ 6 files changed, 97 insertions(+), 26 deletions(-) create mode 100644 application/src/main/java/org/togetherjava/tjbot/features/utils/ConnectionFailedException.java diff --git a/application/src/main/java/org/togetherjava/tjbot/config/JShellConfig.java b/application/src/main/java/org/togetherjava/tjbot/config/JShellConfig.java index 850ca9f4c3..91b85eb2d8 100644 --- a/application/src/main/java/org/togetherjava/tjbot/config/JShellConfig.java +++ b/application/src/main/java/org/togetherjava/tjbot/config/JShellConfig.java @@ -1,10 +1,9 @@ package org.togetherjava.tjbot.config; -import com.linkedin.urls.Url; import org.togetherjava.tjbot.features.utils.RateLimiter; -import java.net.MalformedURLException; +import java.util.Objects; /** * JShell config. @@ -20,18 +19,14 @@ public record JShellConfig(String baseUrl, int rateLimitWindowSeconds, /** * Creates a JShell config. * - * @param baseUrl the base url of the JShell REST API, must be valid + * @param baseUrl the base url of the JShell REST API, must be not null * @param rateLimitWindowSeconds the number of seconds of the {@link RateLimiter rate limiter} * for jshell commands and code actions, must be higher than 0 * @param rateLimitRequestsInWindow the number of requests of the {@link RateLimiter rate * limiter} for jshell commands and code actions, must be higher than 0 */ public JShellConfig { - try { - Url.create(baseUrl); - } catch (MalformedURLException e) { - throw new IllegalArgumentException(e); - } + Objects.requireNonNull(baseUrl); if (rateLimitWindowSeconds < 0) { throw new IllegalArgumentException( "Illegal rateLimitWindowSeconds : " + rateLimitWindowSeconds); diff --git a/application/src/main/java/org/togetherjava/tjbot/features/code/EvalCodeCommand.java b/application/src/main/java/org/togetherjava/tjbot/features/code/EvalCodeCommand.java index 1dfd6a7f8c..ad6cc91a3c 100644 --- a/application/src/main/java/org/togetherjava/tjbot/features/code/EvalCodeCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/features/code/EvalCodeCommand.java @@ -6,6 +6,7 @@ import org.togetherjava.tjbot.features.jshell.JShellEval; import org.togetherjava.tjbot.features.utils.CodeFence; import org.togetherjava.tjbot.features.utils.Colors; +import org.togetherjava.tjbot.features.utils.ConnectionFailedException; import org.togetherjava.tjbot.features.utils.RequestFailedException; /** @@ -34,7 +35,7 @@ public MessageEmbed apply(CodeFence codeFence) { } try { return jshellEval.evaluateAndRespond(null, codeFence.code(), false, false); - } catch (RequestFailedException e) { + } catch (RequestFailedException | ConnectionFailedException e) { return new EmbedBuilder().setColor(Colors.ERROR_COLOR) .setDescription("Request failed: " + e.getMessage()) .build(); diff --git a/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellCommand.java b/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellCommand.java index 920d25fc90..6ae47e40d3 100644 --- a/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellCommand.java @@ -21,6 +21,7 @@ import org.togetherjava.tjbot.features.SlashCommandAdapter; import org.togetherjava.tjbot.features.jshell.backend.JShellApi; import org.togetherjava.tjbot.features.utils.Colors; +import org.togetherjava.tjbot.features.utils.ConnectionFailedException; import org.togetherjava.tjbot.features.utils.MessageUtils; import org.togetherjava.tjbot.features.utils.RequestFailedException; @@ -157,7 +158,7 @@ private void handleEval(IReplyCallback replyCallback, @Nullable User user, boole .editOriginalEmbeds( jshellEval.evaluateAndRespond(user, code, showCode, startupScript)) .queue(); - } catch (RequestFailedException e) { + } catch (RequestFailedException | ConnectionFailedException e) { interactionHook.editOriginalEmbeds(createUnexpectedErrorEmbed(user, e)).queue(); } }); @@ -184,6 +185,9 @@ private void handleSnippetsCommand(SlashCommandInteractionEvent event) { interactionHook.editOriginalEmbeds(createUnexpectedErrorEmbed(user, e)).queue(); } return; + } catch (ConnectionFailedException e) { + interactionHook.editOriginalEmbeds(createUnexpectedErrorEmbed(user, e)).queue(); + return; } sendSnippets(interactionHook, user, snippets); @@ -259,6 +263,9 @@ private void handleCloseCommand(SlashCommandInteractionEvent event) { event.replyEmbeds(createUnexpectedErrorEmbed(event.getUser(), e)).queue(); } return; + } catch (ConnectionFailedException e) { + event.replyEmbeds(createUnexpectedErrorEmbed(event.getUser(), e)).queue(); + return; } event @@ -280,7 +287,7 @@ private void handleStartupScriptCommand(SlashCommandInteractionEvent event) { .setDescription("```java\n" + startupScript + "```") .build()) .queue(); - } catch (RequestFailedException e) { + } catch (RequestFailedException | ConnectionFailedException e) { event.replyEmbeds(createUnexpectedErrorEmbed(event.getUser(), e)).queue(); } }); @@ -293,7 +300,7 @@ private MessageEmbed createSessionNotFoundErrorEmbed(User user) { .build(); } - private MessageEmbed createUnexpectedErrorEmbed(@Nullable User user, RequestFailedException e) { + private MessageEmbed createUnexpectedErrorEmbed(@Nullable User user, Exception e) { EmbedBuilder embedBuilder = new EmbedBuilder().setColor(Colors.ERROR_COLOR) .setDescription("Request failed: " + e.getMessage()); if (user != null) { diff --git a/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellEval.java b/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellEval.java index 91d46feb14..4bcd27a2c1 100644 --- a/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellEval.java +++ b/application/src/main/java/org/togetherjava/tjbot/features/jshell/JShellEval.java @@ -10,6 +10,7 @@ import org.togetherjava.tjbot.features.jshell.backend.JShellApi; import org.togetherjava.tjbot.features.jshell.backend.dto.JShellResult; import org.togetherjava.tjbot.features.utils.Colors; +import org.togetherjava.tjbot.features.utils.ConnectionFailedException; import org.togetherjava.tjbot.features.utils.RateLimiter; import org.togetherjava.tjbot.features.utils.RequestFailedException; @@ -56,7 +57,7 @@ public JShellApi getApi() { * @throws RequestFailedException if a http error happens */ public MessageEmbed evaluateAndRespond(@Nullable User user, String code, boolean showCode, - boolean startupScript) throws RequestFailedException { + boolean startupScript) throws RequestFailedException, ConnectionFailedException { MessageEmbed rateLimitedMessage = wasRateLimited(user, Instant.now()); if (rateLimitedMessage != null) { return rateLimitedMessage; diff --git a/application/src/main/java/org/togetherjava/tjbot/features/jshell/backend/JShellApi.java b/application/src/main/java/org/togetherjava/tjbot/features/jshell/backend/JShellApi.java index 19eb5527d9..cd961a00c6 100644 --- a/application/src/main/java/org/togetherjava/tjbot/features/jshell/backend/JShellApi.java +++ b/application/src/main/java/org/togetherjava/tjbot/features/jshell/backend/JShellApi.java @@ -1,17 +1,20 @@ package org.togetherjava.tjbot.features.jshell.backend; import com.fasterxml.jackson.databind.ObjectMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.togetherjava.tjbot.features.jshell.backend.dto.JShellResult; import org.togetherjava.tjbot.features.jshell.backend.dto.SnippetList; +import org.togetherjava.tjbot.features.utils.ConnectionFailedException; import org.togetherjava.tjbot.features.utils.RequestFailedException; import org.togetherjava.tjbot.features.utils.ResponseUtils; import org.togetherjava.tjbot.features.utils.UncheckedRequestFailedException; import java.io.IOException; import java.io.UncheckedIOException; +import java.net.ConnectException; import java.net.URI; -import java.net.URISyntaxException; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpRequest.BodyPublishers; @@ -33,6 +36,7 @@ * For more information, check the Together-Java JShell backend project. */ public class JShellApi { + private static final Logger logger = LoggerFactory.getLogger(JShellApi.class); public static final int SESSION_NOT_FOUND = 404; /** * The startup script to use when startup script boolean argument is true. @@ -64,8 +68,11 @@ public JShellApi(ObjectMapper objectMapper, String baseUrl) { * executed at the start of the session * @return the result of the evaluation * @throws RequestFailedException if the status code is not 200 or 204 + * @throws ConnectionFailedException if the connection to the API couldn't be made at the first + * place */ - public JShellResult evalOnce(String code, boolean startupScript) throws RequestFailedException { + public JShellResult evalOnce(String code, boolean startupScript) + throws RequestFailedException, ConnectionFailedException { return send( baseUrl + "single-eval" + (startupScript ? "?startupScriptId=" + STARTUP_SCRIPT_ID : ""), @@ -82,9 +89,11 @@ public JShellResult evalOnce(String code, boolean startupScript) throws RequestF * executed at the start of the session * @return the result of the evaluation * @throws RequestFailedException if the status code is not 200 or 204 + * @throws ConnectionFailedException if the connection to the API couldn't be made at the first + * place */ public JShellResult evalSession(String code, String sessionId, boolean startupScript) - throws RequestFailedException { + throws RequestFailedException, ConnectionFailedException { return send( baseUrl + "eval/" + sessionId + (startupScript ? "?startupScriptId=" + STARTUP_SCRIPT_ID : ""), @@ -99,9 +108,11 @@ public JShellResult evalSession(String code, String sessionId, boolean startupSc * @param includeStartupScript if the startup script should be included in the returned snippets * @return the snippets of the session * @throws RequestFailedException if the status code is not 200 or 204 + * @throws ConnectionFailedException if the connection to the API couldn't be made at the first + * place */ public SnippetList snippetsSession(String sessionId, boolean includeStartupScript) - throws RequestFailedException { + throws RequestFailedException, ConnectionFailedException { return send( baseUrl + "snippets/" + sessionId + "?includeStartupScript=" + includeStartupScript, HttpRequest.newBuilder().GET(), @@ -113,8 +124,11 @@ public SnippetList snippetsSession(String sessionId, boolean includeStartupScrip * * @param sessionId the id of the session to close * @throws RequestFailedException if the status code is not 200 or 204 + * @throws ConnectionFailedException if the connection to the API couldn't be made at the first + * place */ - public void closeSession(String sessionId) throws RequestFailedException { + public void closeSession(String sessionId) + throws RequestFailedException, ConnectionFailedException { send(baseUrl + sessionId, HttpRequest.newBuilder().DELETE(), BodyHandlers.discarding()) .body(); } @@ -125,32 +139,51 @@ public void closeSession(String sessionId) throws RequestFailedException { * * @return the startup script * @throws RequestFailedException if the status code is not 200 or 204 + * @throws ConnectionFailedException if the connection to the API couldn't be made at the first + * place */ - public String startupScript() throws RequestFailedException { + public String startupScript() throws RequestFailedException, ConnectionFailedException { return send(baseUrl + "startup_script/" + STARTUP_SCRIPT_ID, HttpRequest.newBuilder().GET(), BodyHandlers.ofString()).body(); } private HttpResponse send(String url, HttpRequest.Builder builder, BodyHandler body) - throws RequestFailedException { + throws RequestFailedException, ConnectionFailedException { try { - HttpResponse response = httpClient.send(builder.uri(new URI(url)).build(), body); + HttpRequest request = buildRequestWithURI(builder, url); + HttpResponse response = httpClient.send(request, body); if (response.statusCode() == 200 || response.statusCode() == 204) { return response; } - throw new RequestFailedException("Request failed with status: " + response.statusCode(), - response.statusCode()); + throw warn("JShell request failed.", new RequestFailedException( + "Request failed with status: " + response.statusCode(), response.statusCode())); } catch (IOException e) { if (e.getCause() instanceof UncheckedRequestFailedException r) { - throw r.toChecked(); + throw warn("JShell request failed.", r.toChecked()); + } + if (e.getCause() instanceof ConnectException ce) { + throw warn("JShell Connection failed.", + new ConnectionFailedException("Couldn't connect to JShell server.", ce)); } throw new UncheckedIOException(e); - } catch (URISyntaxException e) { - throw new IllegalArgumentException(e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IllegalStateException(e); } } + private HttpRequest buildRequestWithURI(HttpRequest.Builder builder, String url) + throws ConnectionFailedException { + try { + return builder.uri(URI.create(url)).build(); + } catch (IllegalArgumentException ex) { + throw warn("Invalid JShell URI.", + new ConnectionFailedException("Couldn't parse JShell URI.", ex)); + } + } + + private T warn(String message, T exception) { + logger.warn(message, exception); + return exception; + } } diff --git a/application/src/main/java/org/togetherjava/tjbot/features/utils/ConnectionFailedException.java b/application/src/main/java/org/togetherjava/tjbot/features/utils/ConnectionFailedException.java new file mode 100644 index 0000000000..7325cc97eb --- /dev/null +++ b/application/src/main/java/org/togetherjava/tjbot/features/utils/ConnectionFailedException.java @@ -0,0 +1,34 @@ +package org.togetherjava.tjbot.features.utils; + +/** + * Happens when a connection has failed, or the URL was invalid. + */ +public class ConnectionFailedException extends Exception { + + /** + * Constructs a new exception with the specified detail message. The cause is not initialized, + * and may subsequently be initialized by a call to {@link #initCause}. + * + * @param message the detail message. The detail message is saved for later retrieval by the + * {@link #getMessage()} method. + */ + public ConnectionFailedException(String message) { + super(message); + } + + /** + * Constructs a new exception with the specified detail message and cause. + *

+ * Note that the detail message associated with {@code cause} is not automatically + * incorporated in this exception's detail message. + * + * @param message the detail message (which is saved for later retrieval by the + * {@link #getMessage()} method). + * @param cause the cause (which is saved for later retrieval by the {@link #getCause()} + * method). (A {@code null} value is permitted, and indicates that the cause is + * nonexistent or unknown.) + */ + public ConnectionFailedException(String message, Throwable cause) { + super(message, cause); + } +}