Skip to content

Commit bbe85c1

Browse files
OracleLabsAutomationelkorchi
OracleLabsAutomation
authored and
elkorchi
committed
[GR-63513] Backport to 24.2: Improve JS exception message.
PullRequest: js/3469
2 parents 7ae8a3a + 09e581d commit bbe85c1

File tree

10 files changed

+213
-96
lines changed

10 files changed

+213
-96
lines changed

‎graal-js/src/com.oracle.truffle.js.test.external/src/com/oracle/truffle/js/test/external/suite/TestRunnable.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -62,7 +62,7 @@
6262

6363
public abstract class TestRunnable implements Runnable {
6464
private static final int LONG_RUNNING_TEST_SECONDS = 55;
65-
protected static final Pattern EXTERNAL_LAUNCHER_ERROR_PATTERN = Pattern.compile("^(\\w+Error)(?::|\\s+at)");
65+
protected static final Pattern EXTERNAL_LAUNCHER_ERROR_PATTERN = Pattern.compile("^(\\w+Error)(?:: .*)?\\R");
6666
protected static final Pattern EXTERNAL_LAUNCHER_EXCEPTION_PATTERN = Pattern.compile("^([\\w\\.]+Exception):");
6767

6868
private static final ConcurrentMap<Integer, Source[]> HARNESS_SOURCES = new ConcurrentHashMap<>();

‎graal-js/src/com.oracle.truffle.js.test.external/src/com/oracle/truffle/js/test/external/suite/TestSuite.java‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -80,13 +80,11 @@
8080
import java.util.stream.Stream;
8181

8282
import org.graalvm.polyglot.Engine;
83+
import org.graalvm.polyglot.PolyglotException;
84+
import org.graalvm.polyglot.Value;
8385

8486
import com.oracle.truffle.js.runtime.JSConfig;
8587
import com.oracle.truffle.js.runtime.JSContextOptions;
86-
import com.oracle.truffle.js.runtime.Strings;
87-
import com.oracle.truffle.js.runtime.UserScriptException;
88-
import com.oracle.truffle.js.runtime.objects.JSDynamicObject;
89-
import com.oracle.truffle.js.runtime.objects.JSObject;
9088

9189
public abstract class TestSuite {
9290

@@ -533,11 +531,18 @@ public final void logFail(TestFile testFile, String failMessage, Throwable cause
533531
}
534532

535533
private static String getDetailedCause(Throwable cause) {
536-
if (cause instanceof UserScriptException) {
537-
UserScriptException use = (UserScriptException) cause;
538-
Object exceptionObject = use.getErrorObject();
539-
if (exceptionObject instanceof JSDynamicObject) {
540-
return String.valueOf(JSObject.get((JSDynamicObject) exceptionObject, Strings.MESSAGE));
534+
if (cause instanceof PolyglotException polyglotException) {
535+
Value guestObject = polyglotException.getGuestObject();
536+
if (guestObject != null) {
537+
try {
538+
if (guestObject.hasMembers()) {
539+
Value message = guestObject.getMember("message");
540+
if (message != null) {
541+
return message.toString();
542+
}
543+
}
544+
} catch (Exception ignored) {
545+
}
541546
}
542547
}
543548
return "";

‎graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/ErrorMessageTest.java‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.truffle.js.test.builtins;
42+
43+
import org.graalvm.polyglot.Context;
44+
import org.graalvm.polyglot.PolyglotException;
45+
import org.graalvm.polyglot.Value;
46+
import org.junit.Assert;
47+
import org.junit.Test;
48+
49+
import com.oracle.truffle.js.lang.JavaScriptLanguage;
50+
import com.oracle.truffle.js.test.JSTest;
51+
52+
public class ErrorMessageTest {
53+
54+
/**
55+
* GR-63023: Provide a better error message by querying error name and message properties.
56+
*/
57+
@Test
58+
public void testErrorNameAndMessage() {
59+
for (boolean as : new boolean[]{false, true}) {
60+
try (Context context = JSTest.newContextBuilder().build()) {
61+
Value customErrorClass = context.eval(JavaScriptLanguage.ID, """
62+
class CustomError extends Error {
63+
constructor(initialMessage, name, message) {
64+
super(initialMessage ?? undefined);
65+
if (name != undefined) {
66+
this.name = name;
67+
}
68+
if (message != undefined) {
69+
this.message = message;
70+
}
71+
}
72+
}
73+
CustomError;
74+
""");
75+
76+
Value errorObject;
77+
PolyglotException polyglotException;
78+
// override name and message
79+
errorObject = customErrorClass.newInstance("initial message", "ExpectedError", "expected message");
80+
polyglotException = asPolyglotException(errorObject, as);
81+
Assert.assertEquals("ExpectedError: expected message", polyglotException.getMessage());
82+
83+
// override only message
84+
errorObject = customErrorClass.newInstance("initial message", null, "expected message");
85+
polyglotException = asPolyglotException(errorObject, as);
86+
Assert.assertEquals("CustomError: expected message", polyglotException.getMessage());
87+
88+
// override only name
89+
errorObject = customErrorClass.newInstance("initial message", "ExpectedError");
90+
polyglotException = asPolyglotException(errorObject, as);
91+
Assert.assertEquals("ExpectedError: initial message", polyglotException.getMessage());
92+
93+
// override only name, no message
94+
errorObject = customErrorClass.newInstance(null, "ExpectedError");
95+
polyglotException = asPolyglotException(errorObject, as);
96+
Assert.assertEquals("ExpectedError", polyglotException.getMessage());
97+
98+
// override none
99+
errorObject = customErrorClass.newInstance("initial message");
100+
polyglotException = asPolyglotException(errorObject, as);
101+
Assert.assertEquals("CustomError: initial message", polyglotException.getMessage());
102+
}
103+
}
104+
}
105+
106+
private static PolyglotException asPolyglotException(Value exception, boolean as) {
107+
if (as) {
108+
return exception.as(PolyglotException.class);
109+
} else {
110+
try {
111+
exception.throwException();
112+
throw new AssertionError("Expected PolyglotException");
113+
} catch (PolyglotException polyglotException) {
114+
return polyglotException;
115+
}
116+
}
117+
}
118+
}

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/GraalJSException.java‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -543,6 +543,31 @@ public final Object toDisplayString(boolean allowSideEffects) {
543543
return JSRuntime.toDisplayString(this, allowSideEffects);
544544
}
545545

546+
/**
547+
* Builds the display message from the error name (required) and message (optional).
548+
*/
549+
protected static String concatErrorNameAndMessage(String name, String message) {
550+
assert name != null;
551+
return (message == null || message.isEmpty()) ? name : name + ": " + message;
552+
}
553+
554+
protected static String getErrorNameSafe(JSObject errorObj, String name) {
555+
// Try error.name first, error.constructor.name second.
556+
Object nameValue = JSRuntime.getDataProperty(errorObj, JSError.NAME);
557+
if (nameValue instanceof TruffleString nameStr && !nameStr.isEmpty() && !nameStr.equals(Strings.UC_ERROR)) {
558+
return Strings.toJavaString(nameStr);
559+
}
560+
return Strings.toJavaString(JSRuntime.getConstructorName(errorObj, Strings.fromJavaString(name)));
561+
}
562+
563+
protected static String getErrorMessageSafe(JSObject errorObj, String message) {
564+
Object messageValue = JSRuntime.getDataProperty(errorObj, JSError.MESSAGE);
565+
if (messageValue instanceof TruffleString messageStr && !messageStr.isEmpty()) {
566+
return Strings.toJavaString(messageStr);
567+
}
568+
return message;
569+
}
570+
546571
@ImportStatic({JSConfig.class})
547572
@ExportMessage
548573
public static final class IsIdenticalOrUndefined {
@@ -672,8 +697,8 @@ public TruffleString getTypeName(boolean checkGlobal) {
672697
if (thisObject == JSFunction.CONSTRUCT) {
673698
return getFunctionName();
674699
} else if (!JSRuntime.isNullOrUndefined(thisObject) && !global) {
675-
if (JSDynamicObject.isJSDynamicObject(thisObject)) {
676-
return JSRuntime.getConstructorName((JSDynamicObject) thisObject);
700+
if (thisObject instanceof JSObject receiver) {
701+
return JSRuntime.getConstructorName(receiver);
677702
} else if (JSRuntime.isJSPrimitive(thisObject)) {
678703
return JSRuntime.getPrimitiveConstructorName(thisObject);
679704
}

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/JSException.java‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -62,6 +62,7 @@
6262
import com.oracle.truffle.js.runtime.builtins.JSError;
6363
import com.oracle.truffle.js.runtime.builtins.JSFunctionObject;
6464
import com.oracle.truffle.js.runtime.objects.JSDynamicObject;
65+
import com.oracle.truffle.js.runtime.objects.JSObject;
6566
import com.oracle.truffle.js.runtime.objects.JSProperty;
6667
import com.oracle.truffle.js.runtime.objects.Undefined;
6768

@@ -71,7 +72,7 @@
7172
public final class JSException extends GraalJSException {
7273

7374
private final JSErrorType type;
74-
private JSDynamicObject exceptionObj;
75+
private JSObject exceptionObj;
7576
private final JSRealm realm;
7677
private final boolean isIncompleteSource;
7778

@@ -84,7 +85,7 @@ private JSException(JSErrorType type, String message, Throwable cause, Node orig
8485
this.isIncompleteSource = false;
8586
}
8687

87-
private JSException(JSErrorType type, String message, Node originatingNode, JSDynamicObject exceptionObj, JSRealm realm, int stackTraceLimit) {
88+
private JSException(JSErrorType type, String message, Node originatingNode, JSObject exceptionObj, JSRealm realm, int stackTraceLimit) {
8889
super(message, originatingNode, stackTraceLimit);
8990
CompilerAsserts.neverPartOfCompilation("JSException constructor");
9091
this.type = type;
@@ -107,21 +108,21 @@ private JSException(JSErrorType type, String message, Throwable cause, SourceSec
107108
}
108109

109110
@TruffleBoundary
110-
public static JSException createCapture(JSErrorType type, String message, JSDynamicObject exceptionObj, JSRealm realm, int stackTraceLimit, JSDynamicObject skipFramesUpTo, boolean customSkip) {
111+
public static JSException createCapture(JSErrorType type, String message, JSObject exceptionObj, JSRealm realm, int stackTraceLimit, JSDynamicObject skipFramesUpTo, boolean customSkip) {
111112
return fillInStackTrace(new JSException(type, message, null, exceptionObj, realm, stackTraceLimit), true, skipFramesUpTo, customSkip);
112113
}
113114

114115
@TruffleBoundary
115-
public static JSException createCapture(JSErrorType type, String message, JSDynamicObject exceptionObj, JSRealm realm) {
116+
public static JSException createCapture(JSErrorType type, String message, JSObject exceptionObj, JSRealm realm) {
116117
return createCapture(type, message, exceptionObj, realm, getStackTraceLimit(realm), Undefined.instance, false);
117118
}
118119

119-
public static JSException create(JSErrorType type, String message, JSDynamicObject exceptionObj, JSRealm realm) {
120+
public static JSException create(JSErrorType type, String message, JSObject exceptionObj, JSRealm realm) {
120121
return create(type, message, (Node) null, exceptionObj, realm);
121122
}
122123

123124
@TruffleBoundary
124-
public static JSException create(JSErrorType type, String message, Node originatingNode, JSDynamicObject exceptionObj, JSRealm realm) {
125+
public static JSException create(JSErrorType type, String message, Node originatingNode, JSObject exceptionObj, JSRealm realm) {
125126
return fillInStackTrace(new JSException(type, message, originatingNode, exceptionObj, realm, getStackTraceLimit(realm)), false);
126127
}
127128

@@ -170,8 +171,14 @@ public static int getStackTraceLimit(JSRealm realm) {
170171
@Override
171172
@TruffleBoundary
172173
public String getMessage() {
174+
String name = type.name();
173175
String message = getRawMessage();
174-
return (message == null || message.isEmpty()) ? type.name() : type.name() + ": " + message;
176+
var errorObj = getErrorObjectLazy();
177+
if (errorObj != null) {
178+
name = getErrorNameSafe(errorObj, name);
179+
message = getErrorMessageSafe(errorObj, message);
180+
}
181+
return concatErrorNameAndMessage(name, message);
175182
}
176183

177184
public String getRawMessage() {
@@ -183,18 +190,18 @@ public JSErrorType getErrorType() {
183190
}
184191

185192
@Override
186-
public JSDynamicObject getErrorObjectLazy() {
193+
public JSObject getErrorObjectLazy() {
187194
return exceptionObj;
188195
}
189196

190-
public void setErrorObject(JSDynamicObject exceptionObj) {
197+
public void setErrorObject(JSObject exceptionObj) {
191198
this.exceptionObj = exceptionObj;
192199
}
193200

194201
@TruffleBoundary
195202
@Override
196203
public Object getErrorObject() {
197-
JSDynamicObject jserror = exceptionObj;
204+
JSObject jserror = exceptionObj;
198205
if (jserror == null) {
199206
String message = getRawMessage();
200207
exceptionObj = jserror = JSError.createFromJSException(this, this.realm, (message == null) ? "" : message);

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/JSRuntime.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -2422,29 +2422,36 @@ public static int comparePropertyKeys(Object key1, Object key2) {
24222422
}
24232423
}
24242424

2425+
public static TruffleString getConstructorName(JSObject receiver) {
2426+
return getConstructorName(receiver, null);
2427+
}
2428+
24252429
/**
24262430
* Carefully try getting the constructor name, must not throw.
24272431
*/
2428-
public static TruffleString getConstructorName(JSDynamicObject receiver) {
2432+
public static TruffleString getConstructorName(JSObject receiver, TruffleString defaultName) {
24292433
// Try @@toStringTag first
24302434
Object toStringTag = getDataProperty(receiver, Symbol.SYMBOL_TO_STRING_TAG);
24312435
if (toStringTag instanceof TruffleString str) {
24322436
return str;
24332437
}
24342438

24352439
// Try function name of prototype.constructor
2436-
if (!isProxy(receiver)) {
2440+
if (!isProxyLike(receiver)) {
24372441
JSDynamicObject prototype = JSObject.getPrototype(receiver);
24382442
if (prototype != Null.instance) {
24392443
Object constructor = getDataProperty(prototype, JSObject.CONSTRUCTOR);
2440-
if (JSFunction.isJSFunction(constructor)) {
2441-
return JSFunction.getName((JSFunctionObject) constructor);
2444+
if (constructor instanceof JSObject constructorObj) {
2445+
Object name = getDataProperty(constructorObj, Strings.NAME);
2446+
if (name instanceof TruffleString nameStr && !nameStr.isEmpty() && !nameStr.equals(Strings.UC_OBJECT)) {
2447+
return nameStr;
2448+
}
24422449
}
24432450
}
24442451
}
24452452

24462453
// As a last resort, use class name
2447-
return JSObject.getClassName(receiver);
2454+
return defaultName != null ? defaultName : JSObject.getClassName(receiver);
24482455
}
24492456

24502457
public static TruffleString getPrimitiveConstructorName(Object primitive) {
@@ -2466,7 +2473,7 @@ public static TruffleString getPrimitiveConstructorName(Object primitive) {
24662473
public static Object getDataProperty(JSDynamicObject thisObj, Object key) {
24672474
assert JSRuntime.isPropertyKey(key);
24682475
JSDynamicObject current = thisObj;
2469-
while (current != Null.instance && current != null && !isProxy(current)) {
2476+
while (current != Null.instance && current != null && !isProxyLike(current)) {
24702477
PropertyDescriptor desc = JSObject.getOwnProperty(current, key);
24712478
if (desc != null) {
24722479
if (desc.isDataDescriptor()) {
@@ -2480,7 +2487,7 @@ public static Object getDataProperty(JSDynamicObject thisObj, Object key) {
24802487
return null;
24812488
}
24822489

2483-
private static boolean isProxy(JSDynamicObject receiver) {
2490+
private static boolean isProxyLike(JSDynamicObject receiver) {
24842491
return JSProxy.isJSProxy(receiver) || JSAdapter.isJSAdapter(receiver);
24852492
}
24862493

0 commit comments

Comments
 (0)