fix ReadableStream.from() cancel behavior per WPT spec

Author: anonrig

src/workerd/api/streams/standard.c++

@@ -4195,7 +4195,18 @@ jsg::Ref<ReadableStream> ReadableStream::from(
               }));
     },
     .cancel = [generator = rcGenerator.addRef()](jsg::Lock& js, auto reason) mutable {
-      return generator->generator.return_(js, js.v8Ref(reason))
+      bool pedanticWpt = FeatureFlags::get(js).getPedanticWpt();
+      // When pedanticWpt is enabled, follow the spec more strictly:
+      // 1. If return property exists but is not callable, throw TypeError
+      // 2. Propagate exceptions from return() directly (don't call throw_())
+      if (pedanticWpt) {
+        if (generator->generator.hasNonCallableReturn()) {
+          return js.rejectedPromise<void>(
+              js.typeError("property 'return' is not a function"_kj));
+        }
+      }
+
+      return generator->generator.return_(js, js.v8Ref(reason), pedanticWpt)
           .then(js, [generator = kj::mv(generator)](auto& lock, auto) {
         // The generator might produce a value on return and might even want to continue,
         // but the stream has been canceled at this point, so we stop here.

src/workerd/jsg/iterator.h

@@ -173,6 +173,15 @@ class AsyncGenerator final {
     }
   }
 
+  // Returns true if the 'return' property exists on the iterator but is not callable.
+  // Per GetMethod spec, this should result in a TypeError when return_() is called.
+  bool hasNonCallableReturn() const {
+    KJ_IF_SOME(active, maybeActive) {
+      return active.returnExistsButNotCallable;
+    }
+    return false;
+  }
+
   // If nothing is returned, the generator is complete.
   Promise<kj::Maybe<T>> next(Lock& js) {
     KJ_IF_SOME(active, maybeActive) {
@@ -205,7 +214,10 @@ class AsyncGenerator final {
   }
 
   // If nothing is returned, the generator is complete.
-  Promise<kj::Maybe<T>> return_(Lock& js, kj::Maybe<T> maybeValue = kj::none) {
+  // When propagateException is true, rejections from return() are propagated directly
+  // without calling throw_(). This matches spec behavior for ReadableStream.from.
+  Promise<kj::Maybe<T>> return_(
+      Lock& js, kj::Maybe<T> maybeValue = kj::none, bool propagateException = false) {
     KJ_IF_SOME(active, maybeActive) {
       KJ_IF_SOME(return_, active.maybeReturn) {
         auto& selfRef = KJ_ASSERT_NONNULL(maybeSelfRef);
@@ -216,17 +228,21 @@ class AsyncGenerator final {
               ref->runIfAlive([&](AsyncGenerator& self) { self.maybeActive = kj::none; });
             }
             return js.resolvedPromise(kj::mv(result.value));
-          }, [ref = selfRef.addRef()](Lock& js, Value exception) {
+          }, [ref = selfRef.addRef(), propagateException](Lock& js, Value exception) {
             Promise<kj::Maybe<T>> retPromise = nullptr;
-            if (ref->runIfAlive([&](AsyncGenerator& self) {
+            if (!propagateException && ref->runIfAlive([&](AsyncGenerator& self) {
               retPromise = self.throw_(js, kj::mv(exception));
             })) {
               return kj::mv(retPromise);
             }
+            ref->runIfAlive([&](AsyncGenerator& self) { self.maybeActive = kj::none; });
             return js.rejectedPromise<kj::Maybe<T>>(kj::mv(exception));
           });
         }, [&](Value exception) {
           maybeActive = kj::none;
+          if (propagateException) {
+            return js.rejectedPromise<kj::Maybe<T>>(kj::mv(exception));
+          }
           return throw_(js, kj::mv(exception));
         });
       }
@@ -276,13 +292,19 @@ class AsyncGenerator final {
     kj::Maybe<NextSignature> maybeNext;
     kj::Maybe<ReturnSignature> maybeReturn;
     kj::Maybe<ThrowSignature> maybeThrow;
+    // Per GetMethod spec, if property exists but is not callable, we should throw TypeError.
+    // We track this state to defer the error to when return_() is actually called.
+    bool returnExistsButNotCallable = false;
 
     template <typename TypeWrapper>
     Active(Lock& js, JsObject object, TypeWrapper*)
         : maybeNext(tryGetGeneratorFunction<NextSignature, TypeWrapper>(js, object, "next"_kj)),
           maybeReturn(
               tryGetGeneratorFunction<ReturnSignature, TypeWrapper>(js, object, "return"_kj)),
           maybeThrow(tryGetGeneratorFunction<ThrowSignature, TypeWrapper>(js, object, "throw"_kj)) {
+      // Check if return property exists but isn't callable (per GetMethod spec)
+      returnExistsButNotCallable =
+          maybeReturn == kj::none && !object.get(js, "return"_kj).isNullOrUndefined();
     }
     Active(Active&&) = default;
     Active& operator=(Active&&) = default;

src/wpt/streams-test.ts

@@ -422,19 +422,7 @@ export default {
       'Floating point arithmetic must manifest near 0 (total ends up zero)',
     ],
   },
-  'readable-streams/from.any.js': {
-    comment: 'See comments on tests',
-    disabledTests: [
-      // A hanging promise was cancelled
-      'ReadableStream.from: cancel() rejects when return() rejects',
-      'ReadableStream.from: cancel() rejects when return() fulfills with a non-object',
-    ],
-    expectedFailures: [
-      // TODO(soon): This one is a bit pedantic. We ignore the case where return() is not
-      // a method whereas the spec expects us to return a rejected promise in this case.
-      'ReadableStream.from: cancel() rejects when return() is not a method',
-    ],
-  },
+  'readable-streams/from.any.js': {},
   'readable-streams/garbage-collection.any.js': {
     comment: 'See comments on individual tests',
     disabledTests: [