[dart2wasm] Support NullRejectionException.isUndefined

Previously, the implementation worked around boxing limitations
by always setting this field to false. However, because we
control both the callback and the JS code that calls the rejection
function, we can pass more information to the callback to let it
know whether the error was undefined.

Change-Id: Ic19f24ec20d321859eb8184d5c87744f7679cf0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439141
Commit-Queue: Srujan Gaddam <[email protected]>
Reviewed-by: Ömer Ağacan <[email protected]>

Author: srujzs

sdk/lib/_internal/wasm/lib/js_helper.dart

@@ -266,11 +266,22 @@ String typeof(WasmExternRef? object) =>
 String stringify(WasmExternRef? object) =>
     JSStringImpl.fromRefUnchecked(JS<WasmExternRef?>("o => String(o)", object));
 
-void promiseThen(
+/// `Promise.then` call where [failureFunc] can be a JS function that expects
+/// two arguments, the first being the error, and the second being whether the
+/// error was undefined.
+///
+/// The second argument is needed as dart2wasm implicitly converts all JS
+/// `undefined`s to Dart `null` when boxing JS values.
+void promiseThenWithIsUndefined(
   WasmExternRef? promise,
   WasmExternRef? successFunc,
   WasmExternRef? failureFunc,
-) => JS<void>("(p, s, f) => p.then(s, f)", promise, successFunc, failureFunc);
+) => JS<void>(
+  "(p, s, f) => p.then(s, (e) => f(e, e === undefined))",
+  promise,
+  successFunc,
+  failureFunc,
+);
 
 // Currently, `allowInterop` returns a Function type. This is unfortunate for
 // Dart2wasm because it means arbitrary Dart functions can flow to JS util

sdk/lib/_internal/wasm/lib/js_interop_patch.dart

@@ -207,21 +207,20 @@ extension JSPromiseToFuture<T extends JSAny?> on JSPromise<T> {
         completer.completeError(e);
       }
     }.toJS;
-    final error = (JSAny? e) {
-      // TODO(joshualitt): Investigate reifying `JSNull` and `JSUndefined` on
-      // all backends and if it is feasible, or feasible for some limited use
-      // cases, then we should pass [e] directly to `completeError`.
-      // TODO(joshualitt): Use helpers to avoid conflating `null` and `JSNull` /
-      // `JSUndefined`.
+    final error = (JSAny? e, bool isUndefined) {
+      // `e` is null when the original error is either JS `null` or JS
+      // `undefined`.
       if (e == null) {
-        // Note that we pass false as a default. It's not currently possible to
-        // be able to differentiate between null and undefined.
-        completer.completeError(js_util.NullRejectionException(false));
+        completer.completeError(js_util.NullRejectionException(isUndefined));
         return;
       }
       completer.completeError(e);
     }.toJS;
-    promiseThen(toExternRef, success.toExternRef, error.toExternRef);
+    js_helper.promiseThenWithIsUndefined(
+      toExternRef,
+      success.toExternRef,
+      error.toExternRef,
+    );
     return completer.future;
   }
 }

sdk/lib/_internal/wasm/lib/js_util_patch.dart

@@ -171,21 +171,21 @@ Future<T> promiseToFuture<T>(Object jsPromise) {
     final r = dartifyRaw(jsValue.toExternRef);
     return completer.complete(r as FutureOr<T>?);
   }).toJS;
-  final error = ((JSAny? jsError) {
-    // Note that `completeError` expects a non-nullable error regardless of
-    // whether null-safety is enabled, so a `NullRejectionException` is always
-    // provided if the error is `null` or `undefined`.
-    // TODO(joshualitt): At this point `undefined` has been replaced with `null`
-    // so we cannot tell them apart. In the future we should reify `undefined`
-    // in Dart.
+  final error = (JSAny? jsError, bool isUndefined) {
+    // `jsError` is null when the original error is either JS `null` or JS
+    // `undefined`.
     final e = dartifyRaw(jsError.toExternRef);
     if (e == null) {
-      return completer.completeError(NullRejectionException(false));
+      completer.completeError(NullRejectionException(isUndefined));
+      return;
     }
-    return completer.completeError(e);
-  }).toJS;
-
-  promiseThen(jsifyRaw(jsPromise), success.toExternRef, error.toExternRef);
+    completer.completeError(e);
+  }.toJS;
+  promiseThenWithIsUndefined(
+    jsifyRaw(jsPromise),
+    success.toExternRef,
+    error.toExternRef,
+  );
   return completer.future;
 }
 

tests/lib/js/static_interop_test/js_types_test.dart

@@ -683,6 +683,7 @@ Future<void> asyncTests() async {
       Expect.fail('Expected rejected promise to throw.');
     } catch (e) {
       Expect.isTrue(e is NullRejectionException);
+      Expect.equals(rejectWithNull, !(e as NullRejectionException).isUndefined);
     }
   }
 

tests/web/wasm/js_util_test.dart

@@ -355,7 +355,8 @@ Future<void> promiseToFutureTest() async {
     globalThis.getResolvedPromise = function() {
       return resolvedPromise;
     }
-    //globalThis.nullRejectedPromise = Promise.reject(null);
+    globalThis.nullRejectedPromise = new Promise((resolve, reject) => reject(null));
+    globalThis.undefinedRejectedPromise = new Promise((resolve, reject) => reject(undefined));
   ''');
 
   // Test resolved
@@ -391,15 +392,26 @@ Future<void> promiseToFutureTest() async {
   }
 
   // Test rejecting promise with null should trigger an exception.
-  // TODO(joshualitt): Fails with an illegal cast.
-  // {
-  //   Future f = promiseToFuture(getProperty(gt, 'nullRejectedPromise'));
-  //   f.then((_) { Expect.fail("Expect promise to reject"); }).catchError((e) {
-  //     print('A');
-  //     Expect.isTrue(e is NullRejectionException);
-  //   });
-  //   await f;
-  // }
+  {
+    try {
+      await promiseToFuture(getProperty(gt, 'nullRejectedPromise'));
+      Expect.fail("Expect promise to reject");
+    } catch (e) {
+      Expect.isTrue(e is NullRejectionException);
+      Expect.isFalse((e as NullRejectionException).isUndefined);
+    }
+  }
+
+  // Test rejecting promise with undefined should trigger an exception.
+  {
+    try {
+      await promiseToFuture(getProperty(gt, 'undefinedRejectedPromise'));
+      Expect.fail("Expect promise to reject");
+    } catch (e) {
+      Expect.isTrue(e is NullRejectionException);
+      Expect.isTrue((e as NullRejectionException).isUndefined);
+    }
+  }
 }
 
 @JS('Symbol')