Print non-Python exceptions in TopLevelExceptionHandler only if AlwaysRunExcepthook

eregon · eregon · commit 801ad720d6be · 2024-10-10T16:36:41.000+02:00
* Fixes #298
diff --git a/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/PythonTests.java b/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/PythonTests.java
@@ -28,6 +28,7 @@
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -38,6 +39,7 @@
 import java.io.PrintStream;
 import java.util.Collections;
 import java.util.Map;
+import java.util.function.Consumer;
 
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.Engine;
@@ -134,6 +136,13 @@ public static void assertPrints(String expected, Source code) {
         assertEquals(expected.replaceAll(" at 0x[0-9a-f]*>", " at 0xabcd>"), result.replaceAll(" at 0x[0-9a-f]*>", " at 0xabcd>"));
     }
 
+    public static void assertPrintsToStdErr(String expected, String code) {
+        final ByteArrayOutputStream byteArray = new ByteArrayOutputStream();
+        PythonTests.runScript(new String[0], code, System.out, byteArray);
+        String result = byteArray.toString().replaceAll("\r\n", "\n");
+        assertEquals(expected.replaceAll(" at 0x[0-9a-f]*>", " at 0xabcd>"), result.replaceAll(" at 0x[0-9a-f]*>", " at 0xabcd>"));
+    }
+
     public static Value eval(String code) {
         final ByteArrayOutputStream byteArray = new ByteArrayOutputStream();
         final PrintStream printStream = new PrintStream(byteArray);
@@ -218,16 +227,24 @@ public static Value runScript(Map<String, String> options, String[] args, String
     }
 
     public static void runThrowableScript(String[] args, String source, OutputStream out, OutputStream err) {
-        try {
-            enterContext(args);
-            context.eval(createSource(source));
-        } catch (PolyglotException t) {
+        runThrowableScript(args, source, out, err, e -> {
             try {
                 Value printExc = context.eval(PRINT_EXC_TO_STDERR);
-                printExc.execute(t.getGuestObject());
+                printExc.execute(e.getGuestObject());
             } catch (Throwable ex) {
                 throw new RuntimeException("Error while printing the PolyglotException message to stderr.", ex);
             }
+        });
+    }
+
+    public static void runThrowableScript(String[] args, String source, OutputStream out, OutputStream err,
+                    Consumer<PolyglotException> exceptionHandler) {
+        try {
+            enterContext(args);
+            context.eval(createSource(source));
+            fail("The eval() should throw");
+        } catch (PolyglotException t) {
+            exceptionHandler.accept(t);
         } finally {
             flush(out, err);
             closeContext();
diff --git a/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/builtin/BaseExceptionTest.java b/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/builtin/BaseExceptionTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
diff --git a/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/interop/JavaInteropTest.java b/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/interop/JavaInteropTest.java
@@ -50,6 +50,7 @@
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.math.BigInteger;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -67,6 +68,7 @@
 import org.graalvm.polyglot.proxy.ProxyHashMap;
 import org.graalvm.polyglot.proxy.ProxyObject;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -93,6 +95,7 @@ public void setUpTest() {
             Builder builder = Context.newBuilder();
             builder.allowExperimentalOptions(true);
             builder.allowAllAccess(true);
+            builder.option("engine.WarnInterpreterOnly", "false");
             builder.out(out);
             builder.err(err);
             context = builder.build();
@@ -197,6 +200,24 @@ public void truffleMethodExport() {
             assertTrue(main.canExecute());
         }
 
+        // From https://github.com/oracle/graalpython/issues/298
+        @Test
+        public void testHostException() {
+            try {
+                context.eval(createSource("import java; java.math.BigInteger.ONE.divide(java.math.BigInteger.ZERO)"));
+                fail();
+            } catch (PolyglotException e) {
+                Assert.assertTrue(e.isHostException());
+                Assert.assertTrue(e.asHostException() instanceof ArithmeticException);
+                Assert.assertTrue(e.getMessage(), e.getMessage().contains("divide by zero"));
+            }
+
+            String outString = out.toString(StandardCharsets.UTF_8);
+            String errString = err.toString(StandardCharsets.UTF_8);
+            Assert.assertTrue(outString, outString.isEmpty());
+            Assert.assertTrue(errString, errString.isEmpty());
+        }
+
         @Test
         public void javaArraySet() {
             String source = "import java\n" +
diff --git a/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/module/PosixTest.java b/graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/module/PosixTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2023, Oracle and/or its affiliates.
+ * Copyright (c) 2017, 2024, Oracle and/or its affiliates.
  * Copyright (c) 2013, Regents of the University of California
  *
  * All rights reserved.
@@ -25,9 +25,9 @@
  */
 package com.oracle.graal.python.test.integration.module;
 
-import static com.oracle.graal.python.test.integration.PythonTests.assertLastLineError;
 import static com.oracle.graal.python.test.integration.PythonTests.assertLastLineErrorContains;
 import static com.oracle.graal.python.test.integration.PythonTests.assertPrints;
+import static com.oracle.graal.python.test.integration.PythonTests.assertPrintsToStdErr;
 import static com.oracle.graal.python.test.integration.Utils.IS_WINDOWS;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -177,7 +177,7 @@ public void stdout() {
     @Test
     public void stderr() {
         assumeFalse(IS_WINDOWS);
-        assertLastLineError("error\n", "import sys; sys.stderr.write('error\\n')");
+        assertPrintsToStdErr("error\n", "import sys; sys.stderr.write('error\\n')");
     }
 
     @Test
@@ -189,7 +189,7 @@ public void printToStdout() {
     @Test
     public void printToStderr() {
         assumeFalse(IS_WINDOWS);
-        assertLastLineError("1-2...", "import sys; print('1', '2', sep='-', file=sys.stderr, end='...', flush=True)");
+        assertPrintsToStdErr("1-2...", "import sys; print('1', '2', sep='-', file=sys.stderr, end='...', flush=True)");
     }
 
     @Test
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/exception/TopLevelExceptionHandler.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/exception/TopLevelExceptionHandler.java
@@ -213,16 +213,18 @@ private void handleJavaException(Throwable e) {
         if (pe != null) {
             throw handlePythonException(pe);
         }
-        try {
-            boolean exitException = InteropLibrary.getUncached().isException(e) && InteropLibrary.getUncached().getExceptionType(e) == ExceptionType.EXIT;
-            if (!exitException) {
-                ExceptionUtils.printPythonLikeStackTrace(getContext(), e);
-                if (PythonOptions.shouldPrintJavaStacktrace(getPythonLanguage(), e)) {
-                    e.printStackTrace();
+        if (getContext().getOption(PythonOptions.AlwaysRunExcepthook)) {
+            try {
+                boolean exitException = InteropLibrary.getUncached().isException(e) && InteropLibrary.getUncached().getExceptionType(e) == ExceptionType.EXIT;
+                if (!exitException) {
+                    ExceptionUtils.printPythonLikeStackTrace(getContext(), e);
+                    if (PythonOptions.shouldPrintJavaStacktrace(getPythonLanguage(), e)) {
+                        e.printStackTrace();
+                    }
                 }
+            } catch (UnsupportedMessageException unsupportedMessageException) {
+                throw CompilerDirectives.shouldNotReachHere();
             }
-        } catch (UnsupportedMessageException unsupportedMessageException) {
-            throw CompilerDirectives.shouldNotReachHere();
         }
     }
 
diff --git a/mx.graalpython/suite.py b/mx.graalpython/suite.py
@@ -453,7 +453,7 @@
             "spotbugsIgnoresGenerated": True,
         },
 
-        # GRAALPYTHON TESTS
+        # GRAALPYTHON_UNIT_TESTS
         "com.oracle.graal.python.test": {
             "subDir": "graalpython",
             "sourceDirs": ["src"],

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`*`
`5`	`5`	`* The Universal Permissive License (UPL), Version 1.0`
Original file line number	Diff line number	Diff line change
`@@ -213,16 +213,18 @@ private void handleJavaException(Throwable e) {`
`213`	`213`	`if (pe != null) {`
`214`	`214`	`throw handlePythonException(pe);`
`215`	`215`	`}`
`216`		`- try {`
`217`		`- boolean exitException = InteropLibrary.getUncached().isException(e) && InteropLibrary.getUncached().getExceptionType(e) == ExceptionType.EXIT;`
`218`		`- if (!exitException) {`
`219`		`- ExceptionUtils.printPythonLikeStackTrace(getContext(), e);`
`220`		`- if (PythonOptions.shouldPrintJavaStacktrace(getPythonLanguage(), e)) {`
`221`		`- e.printStackTrace();`
	`216`	`+ if (getContext().getOption(PythonOptions.AlwaysRunExcepthook)) {`
	`217`	`+ try {`
	`218`	`+ boolean exitException = InteropLibrary.getUncached().isException(e) && InteropLibrary.getUncached().getExceptionType(e) == ExceptionType.EXIT;`
	`219`	`+ if (!exitException) {`
	`220`	`+ ExceptionUtils.printPythonLikeStackTrace(getContext(), e);`
	`221`	`+ if (PythonOptions.shouldPrintJavaStacktrace(getPythonLanguage(), e)) {`
	`222`	`+ e.printStackTrace();`
	`223`	`+ }`
`222`	`224`	`}`
	`225`	`+ } catch (UnsupportedMessageException unsupportedMessageException) {`
	`226`	`+ throw CompilerDirectives.shouldNotReachHere();`
`223`	`227`	`}`
`224`		`- } catch (UnsupportedMessageException unsupportedMessageException) {`
`225`		`- throw CompilerDirectives.shouldNotReachHere();`
`226`	`228`	`}`
`227`	`229`	`}`
`228`	`230`