JSRPC: Make object disposer non-enumerable.

This makes `assert.deepEqual()` work on RPC results, without having to delete the disposer first.
cloudflare · Sep 20, 2024 · c289362 · c289362
1 parent 268dd00
commit c289362
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 11 deletions.
diff --git a/src/workerd/api/tests/js-rpc-test.js b/src/workerd/api/tests/js-rpc-test.js
@@ -1056,12 +1056,6 @@ export let waitUntilWorks = {
   },
 };
 
-function stripDispose(obj) {
-  assert.deepEqual(!!obj[Symbol.dispose], true);
-  delete obj[Symbol.dispose];
-  return obj;
-}
-
 export let serializeRpcPromiseOrProprety = {
   async test(controller, env, ctx) {
     // What happens if we actually try to serialize a JsRpcPromise or JsRpcProperty? Let's make
@@ -1074,7 +1068,7 @@ export let serializeRpcPromiseOrProprety = {
 
     // If we directly return returning a JsRpcPromise, the system automatically awaits it on the
     // server side because it's a thenable.
-    assert.deepEqual(stripDispose(await env.MyService.getRpcPromise(func)), {
+    assert.deepEqual(await env.MyService.getRpcPromise(func), {
       x: 123,
     });
 
@@ -1110,7 +1104,7 @@ export let serializeRpcPromiseOrProprety = {
     // somewhere along the line V8 says "oh look a thenable" and awaits it, before it can be
     // subject to serialization. That's fine.
     assert.deepEqual(
-      stripDispose(await env.MyService.getRemoteNestedRpcPromise(func).value),
+      await env.MyService.getRemoteNestedRpcPromise(func).value,
       { x: 123 }
     );
     await assert.rejects(
@@ -1122,7 +1116,7 @@ export let serializeRpcPromiseOrProprety = {
     );
 
     // The story is similar for a JsRpcProperty -- though the implementation details differ.
-    assert.deepEqual(stripDispose(await env.MyService.getRpcProperty(func)), {
+    assert.deepEqual(await env.MyService.getRpcProperty(func), {
       x: 456,
     });
     assert.strictEqual(await env.MyService.getRpcProperty(func).x, 456);
@@ -1149,7 +1143,7 @@ export let serializeRpcPromiseOrProprety = {
     );
 
     assert.deepEqual(
-      stripDispose(await env.MyService.getRemoteNestedRpcProperty(func).value),
+      await env.MyService.getRemoteNestedRpcProperty(func).value,
       { x: 456 }
     );
     await assert.rejects(

diff --git a/src/workerd/api/worker-rpc.c++ b/src/workerd/api/worker-rpc.c++
@@ -246,7 +246,7 @@ jsg::JsValue deserializeRpcReturnValue(
       v8::Local<v8::Value> func = js.wrapSimpleFunction(js.v8Context(),
           [disposalGroup = kj::mv(disposalGroup)](jsg::Lock&,
               const v8::FunctionCallbackInfo<v8::Value>&) mutable { disposalGroup->disposeAll(); });
-      obj.set(js, js.symbolDispose(), jsg::JsValue(func));
+      obj.setNonEnuremable(js, js.symbolDispose(), jsg::JsValue(func));
     }
   } else {
     // Result wasn't an object, so it must not contain any stubs.

diff --git a/src/workerd/jsg/jsvalue.c++ b/src/workerd/jsg/jsvalue.c++
@@ -67,6 +67,11 @@ void JsObject::setReadOnly(Lock& js, kj::StringPtr name, const JsValue& value) {
       static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete)));
 }
 
+void JsObject::setNonEnuremable(Lock& js, const JsSymbol& name, const JsValue& value) {
+  check(inner->DefineOwnProperty(js.v8Context(), name.inner, value.inner,
+      v8::PropertyAttribute::DontEnum));
+}
+
 JsValue JsObject::get(Lock& js, const JsValue& name) {
   return JsValue(check(inner->Get(js.v8Context(), name.inner)));
 }

diff --git a/src/workerd/jsg/jsvalue.h b/src/workerd/jsg/jsvalue.h
@@ -343,6 +343,7 @@ class JsObject final: public JsBase<v8::Object, JsObject> {
   void set(Lock& js, const JsValue& name, const JsValue& value);
   void set(Lock& js, kj::StringPtr name, const JsValue& value);
   void setReadOnly(Lock& js, kj::StringPtr name, const JsValue& value);
+  void setNonEnuremable(Lock& js, const JsSymbol& name, const JsValue& value);
   JsValue get(Lock& js, const JsValue& name) KJ_WARN_UNUSED_RESULT;
   JsValue get(Lock& js, kj::StringPtr name) KJ_WARN_UNUSED_RESULT;