From 40578bf28a7de958ce626834e3124cea5c890418 Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Fri, 7 Feb 2025 07:40:51 +0300 Subject: [PATCH 1/3] #59 Make BaseKernel#eval() throw runtime EvaluationException --- jjava/pom.xml | 19 +++++++++++++ .../java/org/dflib/jjava/JJavaExtension.java | 6 +---- .../main/java/org/dflib/jjava/JavaKernel.java | 26 +++++++++++------- .../java/org/dflib/jjava/JavaKernelTest.java | 22 +++++++++++++++ .../jjava/jupyter/kernel/BaseKernel.java | 27 +++++++++++++++++-- .../jupyter/kernel/EvaluationException.java | 10 +++++++ pom.xml | 8 ++++++ 7 files changed, 102 insertions(+), 16 deletions(-) create mode 100644 jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java create mode 100644 jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java diff --git a/jjava/pom.xml b/jjava/pom.xml index a326bf6..411a65d 100644 --- a/jjava/pom.xml +++ b/jjava/pom.xml @@ -41,6 +41,16 @@ testcontainers test + + org.mockito + mockito-core + test + + + org.mockito + mockito-junit-jupiter + test + org.slf4j slf4j-simple @@ -189,6 +199,15 @@ + + org.apache.maven.plugins + maven-surefire-plugin + + + --add-opens jdk.jshell/jdk.jshell=ALL-UNNAMED + + + diff --git a/jjava/src/main/java/org/dflib/jjava/JJavaExtension.java b/jjava/src/main/java/org/dflib/jjava/JJavaExtension.java index 5ed5151..84de8b4 100644 --- a/jjava/src/main/java/org/dflib/jjava/JJavaExtension.java +++ b/jjava/src/main/java/org/dflib/jjava/JJavaExtension.java @@ -25,10 +25,6 @@ public class JJavaExtension implements Extension { @Override public void install(BaseKernel kernel) { - try { - kernel.eval(STARTUP_SCRIPT); - } catch (Exception e) { - throw new RuntimeException(e); - } + kernel.eval(STARTUP_SCRIPT); } } diff --git a/jjava/src/main/java/org/dflib/jjava/JavaKernel.java b/jjava/src/main/java/org/dflib/jjava/JavaKernel.java index b84c9c8..90392b4 100644 --- a/jjava/src/main/java/org/dflib/jjava/JavaKernel.java +++ b/jjava/src/main/java/org/dflib/jjava/JavaKernel.java @@ -39,6 +39,7 @@ import org.dflib.jjava.execution.MagicsSourceTransformer; import org.dflib.jjava.jupyter.Extension; import org.dflib.jjava.jupyter.kernel.BaseKernel; +import org.dflib.jjava.jupyter.kernel.EvaluationException; import org.dflib.jjava.jupyter.kernel.LanguageInfo; import org.dflib.jjava.jupyter.kernel.ReplacementOptions; import org.dflib.jjava.jupyter.kernel.display.DisplayData; @@ -210,7 +211,9 @@ public boolean autoLoadExtensions() { } @Override - public List formatError(Exception e) { + public List formatError(Throwable e) { + if (e instanceof EvaluationException) + return formatError(e.getCause()); if (e instanceof CompilationException) { return formatCompilationException((CompilationException) e); } else if (e instanceof IncompleteSourceException) { @@ -338,15 +341,20 @@ public Object evalRaw(String expr) throws Exception { } @Override - public DisplayData eval(String expr) throws Exception { - Object result = this.evalRaw(expr); - - if (result != null) - return result instanceof DisplayData - ? (DisplayData) result - : this.getRenderer().render(result); + public DisplayData eval(String expr) { + Object result; + try { + result = this.evalRaw(expr); + } catch (Exception e) { + throw new EvaluationException(e); + } - return null; + if (result == null) { + return null; + } + return result instanceof DisplayData + ? (DisplayData) result + : this.getRenderer().render(result); } @Override diff --git a/jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java b/jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java new file mode 100644 index 0000000..dceef02 --- /dev/null +++ b/jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java @@ -0,0 +1,22 @@ +package org.dflib.jjava; + +import org.dflib.jjava.jupyter.kernel.EvaluationException; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class JavaKernelTest { + + @Test + public void eval_throwsRuntimeException() throws Exception { + JavaKernel kernel = new JavaKernel("0"); + String expr = "invalid expression"; + + JavaKernel mockKernel = mock(JavaKernel.class); + when(mockKernel.evalRaw(expr)).thenThrow(new Exception("Evaluation error")); + + assertThrows(EvaluationException.class, () -> kernel.eval(expr)); + } +} diff --git a/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java b/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java index 864cf2c..0a1298d 100644 --- a/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java +++ b/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java @@ -161,7 +161,30 @@ public HistoryManager getHistoryManager() { return null; } - public abstract DisplayData eval(String expr) throws Exception; + /** + * Evaluates a code expression in the kernel's language environment and returns the result + * as display data. This is the core evaluation method called when executing code cells + * in a Jupyter notebook. + * + *

The implementation should: + *

    + *
  • Parse and evaluate the provided expression string
    • + *
  • Convert the evaluation result into appropriate display data formats
    • + *
  • Handle any language-specific evaluation context/scope
    • + *
+ * + *

The returned {@link DisplayData} can contain multiple representations of the result + * (e.g. text/plain, text/html, image/png) to allow rich display in the notebook. + * Return null if the expression produces no displayable result. + * + * @param expr The code expression to evaluate as received from the Jupyter frontend + * + * @return A {@link DisplayData} object containing the evaluation result in one or more + * MIME formats, or null if there is no displayable result + * + * @throws EvaluationException If an error occurs during expression evaluation + */ + public abstract DisplayData eval(String expr); /** * Inspect the code to get things such as documentation for a function. This is @@ -283,7 +306,7 @@ public void interrupt() { * not include strings with newlines but rather separate strings each to go on a * new line. */ - public List formatError(Exception e) { + public List formatError(Throwable e) { List lines = new LinkedList<>(); lines.add(this.errorStyler.secondary("---------------------------------------------------------------------------")); diff --git a/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java b/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java new file mode 100644 index 0000000..117f031 --- /dev/null +++ b/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java @@ -0,0 +1,10 @@ +package org.dflib.jjava.jupyter.kernel; + +import java.util.Objects; + +public class EvaluationException extends RuntimeException { + + public EvaluationException(Throwable cause) { + super(Objects.requireNonNull(cause)); + } +} diff --git a/pom.xml b/pom.xml index 9b4e057..6027767 100644 --- a/pom.xml +++ b/pom.xml @@ -53,6 +53,7 @@ 1.3 1.3.0 1.20.2 + 5.3.1 1.7.36 @@ -126,6 +127,13 @@ testcontainers ${testcontainers.version} + + org.mockito + mockito-bom + ${mockito.version} + pom + import + org.slf4j slf4j-simple From 2a18210469a86f3160f6d0cd7d0e1c3dde4d5a6d Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Fri, 14 Mar 2025 16:17:50 +0300 Subject: [PATCH 2/3] #59 Migrate to runtime exceptions, clean up --- .../main/java/org/dflib/jjava/JavaKernel.java | 13 ++--------- .../dflib/jjava/execution/CodeEvaluator.java | 12 +++++----- .../jjava/execution/CompilationException.java | 2 +- .../EvaluationInterruptedException.java | 2 +- .../execution/EvaluationTimeoutException.java | 2 +- .../execution/IncompleteSourceException.java | 2 +- .../execution/JJavaExecutionControl.java | 6 ++--- .../java/org/dflib/jjava/JavaKernelTest.java | 22 ------------------ .../jjava/jupyter/kernel/BaseKernel.java | 23 +++++++++---------- .../jupyter/kernel/EvaluationException.java | 10 -------- 10 files changed, 26 insertions(+), 68 deletions(-) delete mode 100644 jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java delete mode 100644 jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java diff --git a/jjava/src/main/java/org/dflib/jjava/JavaKernel.java b/jjava/src/main/java/org/dflib/jjava/JavaKernel.java index 90392b4..6546309 100644 --- a/jjava/src/main/java/org/dflib/jjava/JavaKernel.java +++ b/jjava/src/main/java/org/dflib/jjava/JavaKernel.java @@ -39,7 +39,6 @@ import org.dflib.jjava.execution.MagicsSourceTransformer; import org.dflib.jjava.jupyter.Extension; import org.dflib.jjava.jupyter.kernel.BaseKernel; -import org.dflib.jjava.jupyter.kernel.EvaluationException; import org.dflib.jjava.jupyter.kernel.LanguageInfo; import org.dflib.jjava.jupyter.kernel.ReplacementOptions; import org.dflib.jjava.jupyter.kernel.display.DisplayData; @@ -212,8 +211,6 @@ public boolean autoLoadExtensions() { @Override public List formatError(Throwable e) { - if (e instanceof EvaluationException) - return formatError(e.getCause()); if (e instanceof CompilationException) { return formatCompilationException((CompilationException) e); } else if (e instanceof IncompleteSourceException) { @@ -334,7 +331,7 @@ private List formatEvaluationInterruptedException(EvaluationInterruptedE return fmt; } - public Object evalRaw(String expr) throws Exception { + public Object evalRaw(String expr) { expr = this.magicsTransformer.transformMagics(expr); return this.evaluator.eval(expr); @@ -342,13 +339,7 @@ public Object evalRaw(String expr) throws Exception { @Override public DisplayData eval(String expr) { - Object result; - try { - result = this.evalRaw(expr); - } catch (Exception e) { - throw new EvaluationException(e); - } - + Object result = this.evalRaw(expr); if (result == null) { return null; } diff --git a/jjava/src/main/java/org/dflib/jjava/execution/CodeEvaluator.java b/jjava/src/main/java/org/dflib/jjava/execution/CodeEvaluator.java index 7bf15d8..a8cf566 100644 --- a/jjava/src/main/java/org/dflib/jjava/execution/CodeEvaluator.java +++ b/jjava/src/main/java/org/dflib/jjava/execution/CodeEvaluator.java @@ -79,14 +79,14 @@ private SourceCodeAnalysis.CompletionInfo analyzeCompletion(String source) { return this.sourceAnalyzer.analyzeCompletion(source); } - private void init() throws Exception { + private void init() { for (String script : this.startupScripts) eval(script); this.startupScripts.clear(); } - protected Object evalSingle(String code) throws Exception { + protected Object evalSingle(String code) { JJavaExecutionControl executionControl = this.executionControlProvider.getRegisteredControlByID(this.executionControlID); @@ -139,11 +139,11 @@ protected Object evalSingle(String code) throws Exception { case JJavaExecutionControl.EXECUTION_INTERRUPTED_NAME: throw new EvaluationInterruptedException(code.trim()); default: - throw e; + throw new RuntimeException(e); } } - throw e; + throw new RuntimeException(e); } if (!event.status().isDefined()) @@ -154,7 +154,7 @@ protected Object evalSingle(String code) throws Exception { return result; } - public Object eval(String code) throws Exception { + public Object eval(String code) { // The init() method runs some code in the shell to initialize the environment. As such // it is deferred until the first user requested evaluation to cleanly return errors when // they happen. @@ -178,7 +178,7 @@ public Object eval(String code) throws Exception { /** * Try to clean up information linked to a code snippet and the snippet itself */ - private void dropSnippet(Snippet snippet) throws Exception { + private void dropSnippet(Snippet snippet) { JJavaExecutionControl executionControl = this.executionControlProvider.getRegisteredControlByID(this.executionControlID); this.shell.drop(snippet); diff --git a/jjava/src/main/java/org/dflib/jjava/execution/CompilationException.java b/jjava/src/main/java/org/dflib/jjava/execution/CompilationException.java index fee801e..77bf0b4 100644 --- a/jjava/src/main/java/org/dflib/jjava/execution/CompilationException.java +++ b/jjava/src/main/java/org/dflib/jjava/execution/CompilationException.java @@ -25,7 +25,7 @@ import jdk.jshell.SnippetEvent; -public class CompilationException extends Exception { +public class CompilationException extends RuntimeException { private final SnippetEvent badSnippetCompilation; public CompilationException(SnippetEvent badSnippetCompilation) { diff --git a/jjava/src/main/java/org/dflib/jjava/execution/EvaluationInterruptedException.java b/jjava/src/main/java/org/dflib/jjava/execution/EvaluationInterruptedException.java index ffcb6a5..5b2919c 100644 --- a/jjava/src/main/java/org/dflib/jjava/execution/EvaluationInterruptedException.java +++ b/jjava/src/main/java/org/dflib/jjava/execution/EvaluationInterruptedException.java @@ -23,7 +23,7 @@ */ package org.dflib.jjava.execution; -public class EvaluationInterruptedException extends Exception { +public class EvaluationInterruptedException extends RuntimeException { private final String source; public EvaluationInterruptedException(String source) { diff --git a/jjava/src/main/java/org/dflib/jjava/execution/EvaluationTimeoutException.java b/jjava/src/main/java/org/dflib/jjava/execution/EvaluationTimeoutException.java index c6622a8..18ac8fb 100644 --- a/jjava/src/main/java/org/dflib/jjava/execution/EvaluationTimeoutException.java +++ b/jjava/src/main/java/org/dflib/jjava/execution/EvaluationTimeoutException.java @@ -25,7 +25,7 @@ import java.util.concurrent.TimeUnit; -public class EvaluationTimeoutException extends Exception { +public class EvaluationTimeoutException extends RuntimeException { private final long duration; private final TimeUnit unit; private final String source; diff --git a/jjava/src/main/java/org/dflib/jjava/execution/IncompleteSourceException.java b/jjava/src/main/java/org/dflib/jjava/execution/IncompleteSourceException.java index 36895d7..6b5ba08 100644 --- a/jjava/src/main/java/org/dflib/jjava/execution/IncompleteSourceException.java +++ b/jjava/src/main/java/org/dflib/jjava/execution/IncompleteSourceException.java @@ -23,7 +23,7 @@ */ package org.dflib.jjava.execution; -public class IncompleteSourceException extends Exception { +public class IncompleteSourceException extends RuntimeException { private final String source; public IncompleteSourceException(String source) { diff --git a/jjava/src/main/java/org/dflib/jjava/execution/JJavaExecutionControl.java b/jjava/src/main/java/org/dflib/jjava/execution/JJavaExecutionControl.java index 7c43087..2ddd96c 100644 --- a/jjava/src/main/java/org/dflib/jjava/execution/JJavaExecutionControl.java +++ b/jjava/src/main/java/org/dflib/jjava/execution/JJavaExecutionControl.java @@ -103,7 +103,7 @@ public Object takeResult(String key) { return result == NULL ? null : result; } - private Object execute(String key, Method doitMethod) throws TimeoutException, Exception { + private Object execute(String key, Method doitMethod) throws Exception { Future runningTask = this.executor.submit(() -> doitMethod.invoke(null)); this.running.put(key, runningTask); @@ -137,7 +137,7 @@ private Object execute(String key, Method doitMethod) throws TimeoutException, E else if (cause instanceof SPIResolutionException) throw new ResolutionException(((SPIResolutionException) cause).id(), cause.getStackTrace()); else - throw new UserException(String.valueOf(cause.getMessage()), String.valueOf(cause.getClass().getName()), cause.getStackTrace()); + throw new UserException(String.valueOf(cause.getMessage()), cause.getClass().getName(), cause.getStackTrace()); } catch (TimeoutException e) { throw new UserException( String.format("Execution timed out after configured timeout of %d %s.", this.timeoutTime, this.timeoutUnit.toString().toLowerCase()), @@ -199,7 +199,7 @@ protected Class findClass(String name) throws ClassNotFoundException { return loaderDelegate.findClass(name); } - void unloadClass(String className) throws ClassNotFoundException { + void unloadClass(String className) { this.loaderDelegate.unloadClass(className); } diff --git a/jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java b/jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java deleted file mode 100644 index dceef02..0000000 --- a/jjava/src/test/java/org/dflib/jjava/JavaKernelTest.java +++ /dev/null @@ -1,22 +0,0 @@ -package org.dflib.jjava; - -import org.dflib.jjava.jupyter.kernel.EvaluationException; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class JavaKernelTest { - - @Test - public void eval_throwsRuntimeException() throws Exception { - JavaKernel kernel = new JavaKernel("0"); - String expr = "invalid expression"; - - JavaKernel mockKernel = mock(JavaKernel.class); - when(mockKernel.evalRaw(expr)).thenThrow(new Exception("Evaluation error")); - - assertThrows(EvaluationException.class, () -> kernel.eval(expr)); - } -} diff --git a/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java b/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java index 0a1298d..22f6fe3 100644 --- a/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java +++ b/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java @@ -59,7 +59,9 @@ public abstract class BaseKernel { protected final AtomicInteger executionCount = new AtomicInteger(1); - protected static final Map KERNEL_META = ((Supplier>) () -> { + protected static final Map KERNEL_META; + + static { Map meta = null; InputStream metaStream = BaseKernel.class.getClassLoader().getResourceAsStream("kernel-metadata.json"); @@ -83,8 +85,8 @@ public abstract class BaseKernel { meta.put("project", "unknown"); } - return meta; - }).get(); + KERNEL_META = meta; + } private final JupyterIO io; private boolean shouldReplaceStdStreams; @@ -181,8 +183,6 @@ public HistoryManager getHistoryManager() { * * @return A {@link DisplayData} object containing the evaluation result in one or more * MIME formats, or null if there is no displayable result - * - * @throws EvaluationException If an error occurs during expression evaluation */ public abstract DisplayData eval(String expr); @@ -201,10 +201,9 @@ public HistoryManager getHistoryManager() { * * @return an output bundle for displaying the documentation or null if nothing is found * - * @throws Exception if the code cannot be inspected for some reason (such as it not - * compiling) + * @throws RuntimeException if the code cannot be inspected for some reason (such as it not compiling) */ - public DisplayData inspect(String code, int at, boolean extraDetail) throws Exception { + public DisplayData inspect(String code, int at, boolean extraDetail) { return null; } @@ -227,11 +226,11 @@ public DisplayData inspect(String code, int at, boolean extraDetail) throws Exce * @return the replacement options containing a list of replacement texts and a * source range to overwrite with a user selected replacement from the list * - * @throws Exception if code cannot be completed due to code compilation issues, or - * similar. This should not be thrown if not replacements are available but rather just - * an empty replacements returned. + * @throws RuntimeException if code cannot be completed due to code compilation issues, or similar. + * This should not be thrown if not replacements are available but rather just + * an empty replacements returned. */ - public ReplacementOptions complete(String code, int at) throws Exception { + public ReplacementOptions complete(String code, int at) { return null; } diff --git a/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java b/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java deleted file mode 100644 index 117f031..0000000 --- a/jupyter-jvm-basekernel/src/main/java/org/dflib/jjava/jupyter/kernel/EvaluationException.java +++ /dev/null @@ -1,10 +0,0 @@ -package org.dflib.jjava.jupyter.kernel; - -import java.util.Objects; - -public class EvaluationException extends RuntimeException { - - public EvaluationException(Throwable cause) { - super(Objects.requireNonNull(cause)); - } -} From 6edef6e81cad4263d9aa4688f257f23c6a3735fe Mon Sep 17 00:00:00 2001 From: Mikhail Dzianishchyts Date: Fri, 14 Mar 2025 16:29:17 +0300 Subject: [PATCH 3/3] #59 Rollback pom.xml --- jjava/pom.xml | 10 ---------- pom.xml | 8 -------- 2 files changed, 18 deletions(-) diff --git a/jjava/pom.xml b/jjava/pom.xml index 411a65d..d14635b 100644 --- a/jjava/pom.xml +++ b/jjava/pom.xml @@ -41,16 +41,6 @@ testcontainers test - - org.mockito - mockito-core - test - - - org.mockito - mockito-junit-jupiter - test - org.slf4j slf4j-simple diff --git a/pom.xml b/pom.xml index 6027767..9b4e057 100644 --- a/pom.xml +++ b/pom.xml @@ -53,7 +53,6 @@ 1.3 1.3.0 1.20.2 - 5.3.1 1.7.36 @@ -127,13 +126,6 @@ testcontainers ${testcontainers.version} - - org.mockito - mockito-bom - ${mockito.version} - pom - import - org.slf4j slf4j-simple