Use wrapJSExceptions() to work around dart-lang/sdk#53105 (#2055)

Author: nex3

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 1.66.1
+
+### JS API
+
+* Fix a bug where Sass compilation could crash in strict mode if passed a
+  callback that threw a string, boolean, number, symbol, or bignum.
+
 ## 1.66.0
 
 * **Breaking change:** Drop support for the additional CSS calculations defined

lib/src/importer/js_to_dart/async.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:cli_pkg/js.dart';
 import 'package:node_interop/js.dart';
 import 'package:node_interop/util.dart';
 
@@ -26,8 +27,8 @@ final class JSToDartAsyncImporter extends AsyncImporter {
   JSToDartAsyncImporter(this._canonicalize, this._load);
 
   FutureOr<Uri?> canonicalize(Uri url) async {
-    var result = _canonicalize(
-        url.toString(), CanonicalizeOptions(fromImport: fromImport));
+    var result = wrapJSExceptions(() => _canonicalize(
+        url.toString(), CanonicalizeOptions(fromImport: fromImport)));
     if (isPromise(result)) result = await promiseToFuture(result as Promise);
     if (result == null) return null;
 
@@ -37,7 +38,7 @@ final class JSToDartAsyncImporter extends AsyncImporter {
   }
 
   FutureOr<ImporterResult?> load(Uri url) async {
-    var result = _load(dartToJSUrl(url));
+    var result = wrapJSExceptions(() => _load(dartToJSUrl(url)));
     if (isPromise(result)) result = await promiseToFuture(result as Promise);
     if (result == null) return null;
 

lib/src/importer/js_to_dart/async_file.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:cli_pkg/js.dart';
 import 'package:node_interop/js.dart';
 import 'package:node_interop/util.dart';
 
@@ -32,8 +33,8 @@ final class JSToDartAsyncFileImporter extends AsyncImporter {
   FutureOr<Uri?> canonicalize(Uri url) async {
     if (url.scheme == 'file') return _filesystemImporter.canonicalize(url);
 
-    var result = _findFileUrl(
-        url.toString(), CanonicalizeOptions(fromImport: fromImport));
+    var result = wrapJSExceptions(() => _findFileUrl(
+        url.toString(), CanonicalizeOptions(fromImport: fromImport)));
     if (isPromise(result)) result = await promiseToFuture(result as Promise);
     if (result == null) return null;
     if (!isJSUrl(result)) {

lib/src/importer/js_to_dart/file.dart

@@ -2,6 +2,7 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'package:cli_pkg/js.dart';
 import 'package:node_interop/js.dart';
 
 import '../../importer.dart';
@@ -27,8 +28,8 @@ final class JSToDartFileImporter extends Importer {
   Uri? canonicalize(Uri url) {
     if (url.scheme == 'file') return _filesystemImporter.canonicalize(url);
 
-    var result = _findFileUrl(
-        url.toString(), CanonicalizeOptions(fromImport: fromImport));
+    var result = wrapJSExceptions(() => _findFileUrl(
+        url.toString(), CanonicalizeOptions(fromImport: fromImport)));
     if (result == null) return null;
 
     if (isPromise(result)) {

lib/src/importer/js_to_dart/sync.dart

@@ -2,6 +2,7 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'package:cli_pkg/js.dart';
 import 'package:node_interop/js.dart';
 
 import '../../importer.dart';
@@ -22,8 +23,8 @@ final class JSToDartImporter extends Importer {
   JSToDartImporter(this._canonicalize, this._load);
 
   Uri? canonicalize(Uri url) {
-    var result = _canonicalize(
-        url.toString(), CanonicalizeOptions(fromImport: fromImport));
+    var result = wrapJSExceptions(() => _canonicalize(
+        url.toString(), CanonicalizeOptions(fromImport: fromImport)));
     if (result == null) return null;
     if (isJSUrl(result)) return jsToDartUrl(result as JSUrl);
 
@@ -37,7 +38,7 @@ final class JSToDartImporter extends Importer {
   }
 
   ImporterResult? load(Uri url) {
-    var result = _load(dartToJSUrl(url));
+    var result = wrapJSExceptions(() => _load(dartToJSUrl(url)));
     if (result == null) return null;
 
     if (isPromise(result)) {

lib/src/importer/legacy_node/implementation.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:cli_pkg/js.dart';
 import 'package:js/js.dart';
 import 'package:path/path.dart' as p;
 
@@ -97,7 +98,8 @@ final class NodeImporter {
     // The previous URL is always an absolute file path for filesystem imports.
     var previousString = _previousToString(previous);
     for (var importer in _importers) {
-      if (call2(importer, _renderContext(forImport), url, previousString)
+      if (wrapJSExceptions(() =>
+              call2(importer, _renderContext(forImport), url, previousString))
           case var value?) {
         return _handleImportResult(url, previous, value, forImport);
       }
@@ -205,8 +207,12 @@ final class NodeImporter {
       String previousString, bool forImport) async {
     var completer = Completer<Object>();
 
-    var result = call3(importer, _renderContext(forImport), url, previousString,
-        allowInterop(completer.complete));
+    var result = wrapJSExceptions(() => call3(
+        importer,
+        _renderContext(forImport),
+        url,
+        previousString,
+        allowInterop(completer.complete)));
     if (isUndefined(result)) return await completer.future;
     return result;
   }

lib/src/js/compile.dart

@@ -2,6 +2,7 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'package:cli_pkg/js.dart';
 import 'package:node_interop/js.dart';
 import 'package:node_interop/util.dart' hide futureToPromise;
 import 'package:term_glyph/term_glyph.dart' as glyph;
@@ -274,7 +275,8 @@ List<AsyncCallable> _parseFunctions(Object? functions, {bool asynch = false}) {
     if (!asynch) {
       late Callable callable;
       callable = Callable.fromSignature(signature, (arguments) {
-        var result = (callback as Function)(toJSArray(arguments));
+        var result = wrapJSExceptions(
+            () => (callback as Function)(toJSArray(arguments)));
         if (result is Value) return _simplifyValue(result);
         if (isPromise(result)) {
           throw 'Invalid return value for custom function '
@@ -290,7 +292,8 @@ List<AsyncCallable> _parseFunctions(Object? functions, {bool asynch = false}) {
     } else {
       late AsyncCallable callable;
       callable = AsyncCallable.fromSignature(signature, (arguments) async {
-        var result = (callback as Function)(toJSArray(arguments));
+        var result = wrapJSExceptions(
+            () => (callback as Function)(toJSArray(arguments)));
         if (isPromise(result)) {
           result = await promiseToFuture<Object>(result as Promise);
         }

lib/src/js/legacy.dart

@@ -7,6 +7,7 @@ import 'dart:convert';
 import 'dart:js_util';
 import 'dart:typed_data';
 
+import 'package:cli_pkg/js.dart';
 import 'package:node_interop/js.dart';
 import 'package:path/path.dart' as p;
 
@@ -217,7 +218,8 @@ List<AsyncCallable> _parseFunctions(RenderOptions options, DateTime start,
             scheduleMicrotask(() => currentFiber.run(result));
           })
         ];
-        var result = (callback as JSFunction).apply(context, jsArguments);
+        var result = wrapJSExceptions(
+            () => (callback as JSFunction).apply(context, jsArguments));
         return unwrapValue(isUndefined(result)
             // Run `fiber.yield()` in runZoned() so that Dart resets the current
             // zone once it's done. Otherwise, interweaving fibers can leave
@@ -228,8 +230,9 @@ List<AsyncCallable> _parseFunctions(RenderOptions options, DateTime start,
     } else if (!asynch) {
       result.add(Callable.fromSignature(
           signature.trimLeft(),
-          (arguments) => unwrapValue((callback as JSFunction)
-              .apply(context, arguments.map(wrapValue).toList())),
+          (arguments) => unwrapValue(wrapJSExceptions(() =>
+              (callback as JSFunction)
+                  .apply(context, arguments.map(wrapValue).toList()))),
           requireParens: false));
     } else {
       result.add(
@@ -239,7 +242,8 @@ List<AsyncCallable> _parseFunctions(RenderOptions options, DateTime start,
           ...arguments.map(wrapValue),
           allowInterop(([Object? result]) => completer.complete(result))
         ];
-        var result = (callback as JSFunction).apply(context, jsArguments);
+        var result = wrapJSExceptions(
+            () => (callback as JSFunction).apply(context, jsArguments));
         return unwrapValue(
             isUndefined(result) ? await completer.future : result);
       }, requireParens: false));

pkg/sass_api/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 8.2.1
+
+* No user-visible changes.
+
 ## 8.2.0
 
 * No user-visible changes.

pkg/sass_api/pubspec.yaml

@@ -2,18 +2,18 @@ name: sass_api
 # Note: Every time we add a new Sass AST node, we need to bump the *major*
 # version because it's a breaking change for anyone who's implementing the
 # visitor interface(s).
-version: 8.2.0
+version: 8.2.1
 description: Additional APIs for Dart Sass.
 homepage: https://github.com/sass/dart-sass
 
 environment:
   sdk: ">=3.0.0 <4.0.0"
 
 dependencies:
-  sass: 1.66.0
+  sass: 1.66.1
 
 dev_dependencies:
-  dartdoc: ^5.0.0
+  dartdoc: ^6.0.0
 
 dependency_overrides:
   sass: { path: ../.. }

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.66.0
+version: 1.66.1
 description: A Sass implementation in Dart.
 homepage: https://github.com/sass/dart-sass
 
@@ -14,6 +14,7 @@ dependencies:
   args: ^2.0.0
   async: ^2.5.0
   charcode: ^1.2.0
+  cli_pkg: ^2.5.0
   cli_repl: ^0.2.1
   collection: ^1.16.0
   http: "^1.1.0"
@@ -38,7 +39,6 @@ dependencies:
 dev_dependencies:
   analyzer: ">=5.13.0 <7.0.0"
   archive: ^3.1.2
-  cli_pkg: ^2.4.4
   crypto: ^3.0.0
   dart_style: ^2.0.0
   dartdoc: ^6.0.0