Do not throw ProtocolErrorType.PARAMS for custom functions and importers

bin/dart_sass_embedded.dart

@@ -55,13 +55,8 @@ void main(List<String> args) {
           _decodeImporter(dispatcher, request, importer) ??
           (throw mandatoryError("Importer.importer")));
 
-      var globalFunctions = request.globalFunctions.map((signature) {
-        try {
-          return hostCallable(dispatcher, functions, request.id, signature);
-        } on sass.SassException catch (error) {
-          throw paramsError('CompileRequest.global_functions: $error');
-        }
-      });
+      var globalFunctions = request.globalFunctions.map((signature) =>
+          hostCallable(dispatcher, functions, request.id, signature));
 
       late sass.CompileResult result;
       switch (request.whichInput()) {

lib/src/importer/base.dart

@@ -6,8 +6,6 @@ import 'package:meta/meta.dart';
 import 'package:sass_api/sass_api.dart' as sass;
 
 import '../dispatcher.dart';
-import '../embedded_sass.pb.dart' hide SourceSpan;
-import '../utils.dart';
 
 /// An abstract base class for importers that communicate with the host in some
 /// way.
@@ -28,18 +26,10 @@ abstract class ImporterBase extends sass.Importer {
     try {
       parsedUrl = Uri.parse(url);
     } on FormatException catch (error) {
-      sendAndThrow(paramsError("$field is invalid: $error"));
+      throw "$field is invalid: $error";
     }
 
     if (parsedUrl.scheme.isNotEmpty) return parsedUrl;
-    sendAndThrow(paramsError('$field must be absolute, was "$parsedUrl"'));
-  }
-
-  /// Sends [error] to the remote endpoint, and also throws it so that the Sass
-  /// compilation fails.
-  @protected
-  Never sendAndThrow(ProtocolError error) {
-    dispatcher.sendError(error);
-    throw "Protocol error: ${error.message}";
+    throw '$field must be absolute, was "$parsedUrl"';
   }
 }

lib/src/importer/file.dart

@@ -9,7 +9,6 @@ import 'package:sass_api/sass_api.dart' as sass;
 
 import '../dispatcher.dart';
 import '../embedded_sass.pb.dart' hide SourceSpan;
-import '../utils.dart';
 import 'base.dart';
 
 /// A filesystem importer to use for most implementation details of
@@ -47,8 +46,7 @@ class FileImporter extends ImporterBase {
           var url =
               parseAbsoluteUrl("FileImportResponse.file_url", response.fileUrl);
           if (url.scheme != 'file') {
-            sendAndThrow(paramsError(
-                'FileImportResponse.file_url must be a file: URL, was "$url"'));
+            throw 'FileImportResponse.file_url must be a file: URL, was "$url"';
           }
 
           return _filesystemImporter.canonicalize(url);

test/file_importer_test.dart

@@ -54,6 +54,18 @@ void main() {
           "InboundMessage_CanonicalizeResponse.");
       await process.kill();
     });
+  });
+
+  group("emits a compile failure", () {
+    late OutboundMessage_FileImportRequest request;
+
+    setUp(() async {
+      process.inbound.add(compileString("@import 'other'", importers: [
+        InboundMessage_CompileRequest_Importer()..fileImporterId = 1
+      ]));
+
+      request = getFileImportRequest(await process.outbound.next);
+    });
 
     group("for a FileImportResponse with a URL", () {
       test("that's empty", () async {
@@ -62,7 +74,7 @@ void main() {
             ..id = request.id
             ..fileUrl = ""));
 
-        await _expectImportParamsError(
+        await _expectImportError(
             process, 'FileImportResponse.file_url must be absolute, was ""');
         await process.kill();
       });
@@ -73,7 +85,7 @@ void main() {
             ..id = request.id
             ..fileUrl = "foo"));
 
-        await _expectImportParamsError(
+        await _expectImportError(
             process, 'FileImportResponse.file_url must be absolute, was "foo"');
         await process.kill();
       });
@@ -84,7 +96,7 @@ void main() {
             ..id = request.id
             ..fileUrl = "other:foo"));
 
-        await _expectImportParamsError(process,
+        await _expectImportError(process,
             'FileImportResponse.file_url must be a file: URL, was "other:foo"');
         await process.kill();
       });
@@ -268,14 +280,10 @@ void main() {
   });
 }
 
-/// Asserts that [process] emits a [ProtocolError] params error with the given
+/// Asserts that [process] emits a [CompileFailure] result with the given
 /// [message] on its protobuf stream and causes the compilation to fail.
-Future<void> _expectImportParamsError(
-    EmbeddedProcess process, Object message) async {
-  await expectLater(process.outbound,
-      emits(isProtocolError(errorId, ProtocolErrorType.PARAMS, message)));
-
+Future<void> _expectImportError(EmbeddedProcess process, Object message) async {
   var failure = getCompileFailure(await process.outbound.next);
-  expect(failure.message, equals('Protocol error: $message'));
+  expect(failure.message, equals(message));
   expect(failure.span.text, equals("'other'"));
 }

test/function_test.dart

@@ -21,74 +21,34 @@ void main() {
     _process = await EmbeddedProcess.start();
   });
 
-  group("emits a protocol error for a custom function with a signature", () {
+  group("emits a compile failure for a custom function with a signature", () {
     test("that's empty", () async {
       _process.inbound.add(compileString("a {b: c}", functions: [r""]));
-      await expectParamsError(
-          _process,
-          0,
-          'CompileRequest.global_functions: Error: "" is missing "("\n'
-          '  ╷\n'
-          '1 │ \n'
-          '  │ ^\n'
-          '  ╵\n'
-          '  - 1:1  root stylesheet');
+      await _expectFunctionError(_process, '"" is missing "("');
       await _process.kill();
     });
 
     test("that's just a name", () async {
       _process.inbound.add(compileString("a {b: c}", functions: [r"foo"]));
-      await expectParamsError(
-          _process,
-          0,
-          'CompileRequest.global_functions: Error: "foo" is missing "("\n'
-          '  ╷\n'
-          '1 │ foo\n'
-          '  │ ^^^\n'
-          '  ╵\n'
-          '  - 1:1  root stylesheet');
+      await _expectFunctionError(_process, '"foo" is missing "("');
       await _process.kill();
     });
 
     test("without a closing paren", () async {
       _process.inbound.add(compileString("a {b: c}", functions: [r"foo($bar"]));
-      await expectParamsError(
-          _process,
-          0,
-          'CompileRequest.global_functions: Error: "foo(\$bar" doesn\'t end with ")"\n'
-          '  ╷\n'
-          '1 │ foo(\$bar\n'
-          '  │         ^\n'
-          '  ╵\n'
-          '  - 1:9  root stylesheet');
+      await _expectFunctionError(_process, '"foo(\$bar" doesn\'t end with ")"');
       await _process.kill();
     });
 
     test("with text after the closing paren", () async {
       _process.inbound.add(compileString("a {b: c}", functions: [r"foo() "]));
-      await expectParamsError(
-          _process,
-          0,
-          'CompileRequest.global_functions: Error: "foo() " doesn\'t end with ")"\n'
-          '  ╷\n'
-          '1 │ foo() \n'
-          '  │       ^\n'
-          '  ╵\n'
-          '  - 1:7  root stylesheet');
+      await _expectFunctionError(_process, '"foo() " doesn\'t end with ")"');
       await _process.kill();
     });
 
     test("with invalid arguments", () async {
       _process.inbound.add(compileString("a {b: c}", functions: [r"foo($)"]));
-      await expectParamsError(
-          _process,
-          0,
-          'CompileRequest.global_functions: Error: Expected identifier.\n'
-          '  ╷\n'
-          '1 │ @function foo(\$) {\n'
-          '  │                ^\n'
-          '  ╵\n'
-          '  - 1:16  root stylesheet');
+      await _expectFunctionError(_process, 'Expected identifier.');
       await _process.kill();
     });
   });
@@ -1989,3 +1949,11 @@ Value _hsl(num hue, num saturation, num lightness, double alpha) => Value()
     ..saturation = saturation * 1.0
     ..lightness = lightness * 1.0
     ..alpha = alpha);
+
+/// Asserts that [process] emits a [CompileFailure] result with the given
+/// [message] on its protobuf stream and causes the compilation to fail.
+Future<void> _expectFunctionError(
+    EmbeddedProcess process, Object message) async {
+  var failure = getCompileFailure(await process.outbound.next);
+  expect(failure.message, equals(message));
+}

test/importer_test.dart

@@ -64,7 +64,7 @@ void main() {
   });
 
   group("canonicalization", () {
-    group("emits a protocol error", () {
+    group("emits a compile failure", () {
       test("for a canonicalize response with an empty URL", () async {
         process.inbound.add(compileString("@import 'other'", importers: [
           InboundMessage_CompileRequest_Importer()..importerId = 1
@@ -76,7 +76,7 @@ void main() {
             ..id = request.id
             ..url = ""));
 
-        await _expectImportParamsError(
+        await _expectImportError(
             process, 'CanonicalizeResponse.url must be absolute, was ""');
         await process.kill();
       });
@@ -92,7 +92,7 @@ void main() {
             ..id = request.id
             ..url = "relative"));
 
-        await _expectImportParamsError(process,
+        await _expectImportError(process,
             'CanonicalizeResponse.url must be absolute, was "relative"');
         await process.kill();
       });
@@ -216,7 +216,7 @@ void main() {
   });
 
   group("importing", () {
-    group("emits a protocol error", () {
+    group("emits a compile failure", () {
       test("for an import result with a relative sourceMapUrl", () async {
         process.inbound.add(compileString("@import 'other'", importers: [
           InboundMessage_CompileRequest_Importer()..importerId = 1
@@ -230,7 +230,7 @@ void main() {
             ..success = (InboundMessage_ImportResponse_ImportSuccess()
               ..sourceMapUrl = "relative")));
 
-        await _expectImportParamsError(
+        await _expectImportError(
             process,
             'ImportResponse.success.source_map_url must be absolute, was '
             '"relative"');
@@ -510,14 +510,10 @@ Future<void> _canonicalize(EmbeddedProcess process) async {
       ..url = "custom:other"));
 }
 
-/// Asserts that [process] emits a [ProtocolError] params error with the given
+/// Asserts that [process] emits a [CompileFailure] result with the given
 /// [message] on its protobuf stream and causes the compilation to fail.
-Future<void> _expectImportParamsError(
-    EmbeddedProcess process, Object message) async {
-  await expectLater(process.outbound,
-      emits(isProtocolError(errorId, ProtocolErrorType.PARAMS, message)));
-
+Future<void> _expectImportError(EmbeddedProcess process, Object message) async {
   var failure = getCompileFailure(await process.outbound.next);
-  expect(failure.message, equals('Protocol error: $message'));
+  expect(failure.message, equals(message));
   expect(failure.span.text, equals("'other'"));
 }