Use a common error handler for embedded protocol errors (#2027)

sass · Jun 23, 2023 · 61af9ee · 61af9ee
1 parent 658eb70
commit 61af9ee
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 47 deletions.
diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart
@@ -10,7 +10,6 @@ import 'dart:typed_data';
 import 'package:path/path.dart' as p;
 import 'package:protobuf/protobuf.dart';
 import 'package:sass/sass.dart' as sass;
-import 'package:stack_trace/stack_trace.dart';
 import 'package:stream_channel/stream_channel.dart';
 
 import 'embedded_sass.pb.dart';
@@ -116,24 +115,8 @@ class Dispatcher {
             throw parseError(
                 "Unknown message type: ${message.toDebugString()}");
         }
-      } on ProtocolError catch (error) {
-        // Always set the ID to [errorId] because we're only ever reporting
-        // errors for responses or for [CompileRequest] which has no ID.
-        error.id = errorId;
-        stderr.write("Host caused ${error.type.name.toLowerCase()} error");
-        if (error.id != errorId) stderr.write(" with request ${error.id}");
-        stderr.writeln(": ${error.message}");
-        sendError(error);
-        // PROTOCOL error from https://bit.ly/2poTt90
-        exitCode = 76;
-        _channel.sink.close();
-      } catch (error, stackTrace) {
-        var errorMessage = "$error\n${Chain.forTrace(stackTrace)}";
-        stderr.write("Internal compiler error: $errorMessage");
-        sendError(ProtocolError()
-          ..type = ProtocolErrorType.INTERNAL
-          ..id = errorId
-          ..message = errorMessage);
+      } on ProtocolError catch (error, stackTrace) {
+        sendError(handleError(error, stackTrace));
         _channel.sink.close();
       }
     }).asFuture<void>();

diff --git a/lib/src/embedded/host_callable.dart b/lib/src/embedded/host_callable.dart
@@ -4,7 +4,6 @@
 
 // ignore: deprecated_member_use
 import 'dart:cli';
-import 'dart:io';
 
 import '../callable.dart';
 import '../exception.dart';
@@ -51,11 +50,8 @@ Callable hostCallable(
         case InboundMessage_FunctionCallResponse_Result.notSet:
           throw mandatoryError('FunctionCallResponse.result');
       }
-    } on ProtocolError catch (error) {
-      error.id = errorId;
-      stderr.writeln("Host caused ${error.type.name.toLowerCase()} error: "
-          "${error.message}");
-      dispatcher.sendError(error);
+    } on ProtocolError catch (error, stackTrace) {
+      dispatcher.sendError(handleError(error, stackTrace));
       throw error.message;
     }
   });

diff --git a/lib/src/embedded/isolate_dispatcher.dart b/lib/src/embedded/isolate_dispatcher.dart
@@ -4,13 +4,11 @@
 
 import 'dart:async';
 import 'dart:ffi';
-import 'dart:io';
 import 'dart:isolate';
 import 'dart:typed_data';
 
 import 'package:pool/pool.dart';
 import 'package:protobuf/protobuf.dart';
-import 'package:stack_trace/stack_trace.dart';
 import 'package:stream_channel/isolate_channel.dart';
 import 'package:stream_channel/stream_channel.dart';
 
@@ -199,26 +197,9 @@ class IsolateDispatcher {
   /// responded to, if available.
   void _handleError(Object error, StackTrace stackTrace,
       {int? compilationId, int? messageId}) {
-    if (error is ProtocolError) {
-      error.id = messageId ?? errorId;
-      stderr.write("Host caused ${error.type.name.toLowerCase()} error");
-      if (error.id != errorId) stderr.write(" with request ${error.id}");
-      stderr.writeln(": ${error.message}");
-      sendError(compilationId ?? errorId, error);
-      // PROTOCOL error from https://bit.ly/2poTt90
-      exitCode = 76;
-      _channel.sink.close();
-    } else {
-      var errorMessage = "$error\n${Chain.forTrace(stackTrace)}";
-      stderr.write("Internal compiler error: $errorMessage");
-      sendError(
-          compilationId ?? errorId,
-          ProtocolError()
-            ..type = ProtocolErrorType.INTERNAL
-            ..id = messageId ?? errorId
-            ..message = errorMessage);
-      _channel.sink.close();
-    }
+    sendError(compilationId ?? errorId,
+        handleError(error, stackTrace, messageId: messageId));
+    _channel.sink.close();
   }
 
   /// Sends [message] to the host.

diff --git a/lib/src/embedded/utils.dart b/lib/src/embedded/utils.dart
@@ -2,10 +2,12 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'dart:io';
 import 'dart:typed_data';
 
 import 'package:protobuf/protobuf.dart';
 import 'package:source_span/source_span.dart';
+import 'package:stack_trace/stack_trace.dart';
 import 'package:term_glyph/term_glyph.dart' as term_glyph;
 
 import '../syntax.dart';
@@ -134,3 +136,25 @@ final _compilationIdBuilder = VarintBuilder(32, 'compilation ID');
     _compilationIdBuilder.reset();
   }
 }
+
+/// Wraps error object into ProtocolError, writes error to stderr, and returns the ProtocolError.
+ProtocolError handleError(Object error, StackTrace stackTrace,
+    {int? messageId}) {
+  if (error is ProtocolError) {
+    error.id = messageId ?? errorId;
+    stderr.write("Host caused ${error.type.name.toLowerCase()} error");
+    if (error.id != errorId) stderr.write(" with request ${error.id}");
+    stderr.writeln(": ${error.message}");
+    // PROTOCOL error from https://bit.ly/2poTt90
+    exitCode = 76; // EX_PROTOCOL
+    return error;
+  } else {
+    var errorMessage = "$error\n${Chain.forTrace(stackTrace)}";
+    stderr.write("Internal compiler error: $errorMessage");
+    exitCode = 70; // EX_SOFTWARE
+    return ProtocolError()
+      ..type = ProtocolErrorType.INTERNAL
+      ..id = messageId ?? errorId
+      ..message = errorMessage;
+  }
+}