From c9809eea8627ea86610833f5445c6e586eac66dd Mon Sep 17 00:00:00 2001
From: Hood Chatham <roberthoodchatham@gmail.com>
Date: Mon, 25 Mar 2024 13:15:25 +0100
Subject: [PATCH] Don't throw when calling str on a proxy without a toString
 method (#4574)

Resolves #4569. It still doesn't make sure `str(jsproxy)` never throws. It will throw if:
1. accessing `obj.toString` succeeds and returns a function, but calling the function throws
2. accessing `obj.toString` fails or returns not a function, and `Object.prototype.toString.call` fails.
---
 docs/project/changelog.md |  7 ++++
 src/core/jslib.c          |  5 ++-
 src/core/jsproxy.c        |  3 --
 src/tests/test_jsproxy.py | 72 ++++++++++++++++++++++++++++++++++++---
 4 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/docs/project/changelog.md b/docs/project/changelog.md
index ff4ad5dc0..8ec16519d 100644
--- a/docs/project/changelog.md
+++ b/docs/project/changelog.md
@@ -16,6 +16,13 @@ myst:
 
 ## Unreleased
 
+- {{ Enhancement }} `str(jsproxy)` has been adjusted to not raise an error if
+  `jsproxy.toString` is undefined. Instead, it will use
+  `Object.prototype.toString` in this case. If `jsproxy.toString` is defined and
+  throws or is not defined but `jsproxy[Symbol.toStringTag]` is defined and
+  throws, then `str` will still raise.
+  {pr}`4574`
+
 - {{ Enhancement }} Improved support for stack switching.
   {pr}`4532`, {pr}`4547`
 
diff --git a/src/core/jslib.c b/src/core/jslib.c
index 67d76d62e..124e59781 100644
--- a/src/core/jslib.c
+++ b/src/core/jslib.c
@@ -279,7 +279,10 @@ EM_JS_VAL(JsVal, JsvObject_Values, (JsVal obj), {
 
 EM_JS_VAL(JsVal,
 JsvObject_toString, (JsVal obj), {
-  return obj.toString();
+  if (hasMethod(obj, "toString")) {
+    return obj.toString();
+  }
+  return Object.prototype.toString.call(obj);
 });
 
 
diff --git a/src/core/jsproxy.c b/src/core/jsproxy.c
index a8605af4d..50721ade8 100644
--- a/src/core/jsproxy.c
+++ b/src/core/jsproxy.c
@@ -269,9 +269,6 @@ JsProxy_Repr(PyObject* self)
 {
   JsVal repr = JsvObject_toString(JsProxy_VAL(self));
   if (JsvNull_Check(repr)) {
-    PyErr_Format(PyExc_TypeError,
-                 "Pyodide cannot generate a repr for this Javascript object "
-                 "because it has no 'toString' method");
     return NULL;
   }
   return js2python(repr);
diff --git a/src/tests/test_jsproxy.py b/src/tests/test_jsproxy.py
index 3a22871e1..0aaf89b38 100644
--- a/src/tests/test_jsproxy.py
+++ b/src/tests/test_jsproxy.py
@@ -1286,13 +1286,10 @@ def test_js_id(selenium):
 
 @run_in_pyodide
 def test_object_with_null_constructor(selenium):
-    from unittest import TestCase
-
     from pyodide.code import run_js
 
     o = run_js("Object.create(null)")
-    with TestCase().assertRaises(TypeError):
-        repr(o)
+    assert repr(o) == "[object Object]"
 
 
 @pytest.mark.parametrize("n", [1 << 31, 1 << 32, 1 << 33, 1 << 63, 1 << 64, 1 << 65])
@@ -2557,3 +2554,70 @@ def test_js_proxy_attribute(selenium):
     assert x.c is None
     with pytest.raises(AttributeError):
         x.d  # noqa: B018
+
+
+@run_in_pyodide
+async def test_js_proxy_str(selenium):
+    import re
+
+    import pytest
+
+    from js import Array
+    from pyodide.code import run_js
+    from pyodide.ffi import JsException
+
+    assert (
+        re.sub(r"\s+", " ", str(Array).replace("\n", " "))
+        == "function Array() { [native code] }"
+    )
+    assert str(run_js("[1,2,3]")) == "1,2,3"
+    assert str(run_js("Object.create(null)")) == "[object Object]"
+    mod = await run_js("import('data:text/javascript,')")
+    assert str(mod) == "[object Module]"
+    # accessing toString fails, should fall back to Object.prototype.toString.call
+    x = run_js(
+        """
+        ({
+          get toString() {
+            throw new Error();
+          },
+          [Symbol.toStringTag] : "SomeTag"
+        })
+        """
+    )
+    assert str(x) == "[object SomeTag]"
+    # accessing toString succeeds but toString call throws, let exception propagate
+    x = run_js(
+        """
+        ({
+          toString() {
+            throw new Error("hi!");
+          },
+        })
+        """
+    )
+    with pytest.raises(JsException, match="hi!"):
+        str(x)
+
+    # No toString method, so we fall back to Object.prototype.toString.call
+    # which throws, let error propagate
+    x = run_js(
+        """
+        ({
+          get [Symbol.toStringTag]() {
+            throw new Error("hi!");
+          },
+        });
+        """
+    )
+    with pytest.raises(JsException, match="hi!"):
+        str(x)
+
+    # accessing toString fails, so fall back to Object.prototype.toString.call
+    # which also throws, let error propagate
+    px = run_js("(p = Proxy.revocable({}, {})); p.revoke(); p.proxy")
+    with pytest.raises(
+        JsException,
+        match="revoked",
+    ):
+        str(px)