Skip to content

Commit

Permalink
JShell now allows wrong uri and dead server, will give correct respon…
Browse files Browse the repository at this point in the history
…se to the user and will log them as warns
  • Loading branch information
Alathreon committed Aug 10, 2023
1 parent 01054fc commit 7edd53c
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}
});
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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();
}
});
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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 : ""),
Expand All @@ -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 : ""),
Expand All @@ -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(),
Expand All @@ -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();
}
Expand All @@ -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 <T> HttpResponse<T> send(String url, HttpRequest.Builder builder, BodyHandler<T> body)
throws RequestFailedException {
throws RequestFailedException, ConnectionFailedException {
try {
HttpResponse<T> response = httpClient.send(builder.uri(new URI(url)).build(), body);
HttpRequest request = buildRequestWithURI(builder, url);
HttpResponse<T> 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 extends Exception> T warn(String message, T exception) {
logger.warn(message, exception);
return exception;
}
}
Original file line number Diff line number Diff line change
@@ -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.
* <p>
* Note that the detail message associated with {@code cause} is <i>not</i> 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);
}
}

0 comments on commit 7edd53c

Please sign in to comment.