Tweak Ruby->Go error handling so exit codes match

In legacy Vagrant, any exception raised that's a subclass of
Vagrant::Errors::VagrantError is considered user-facing and so causes
the error message to be printed to the console and the process to use
exit code 1. Anything outside of that causes the process to use exit
code 255. (See `bin/vagrant` for the code.)

Here we mirror that behavior by treating errors that have a
LocalizedMessage as user-facing and those without as unexpected. This
allows the basic virtualbox component to pass in vagrant-spec!

internal/cli/dynamic.go

@@ -101,14 +101,25 @@ func (c *DynamicCommand) Run(args []string) int {
 		} else if !r.RunResult {
 			runErrorStatus := status.FromProto(r.RunError)
 			details := runErrorStatus.Details()
+			userError := false
 			for _, msg := range details {
 				switch m := msg.(type) {
 				case *errdetails.LocalizedMessage:
-					cl.UI().Output("Error: "+m.Message+"\n", terminal.WithErrorStyle())
+					// Errors from Ruby with LocalizedMessages are user-facing,
+					// so can be output directly.
+					userError = true
+					cl.UI().Output(m.Message, terminal.WithErrorStyle())
+					// All user-facing errors from Ruby use a 1 exit code. See
+					// Vagrant::Errors::VagrantError.
+					r.ExitCode = 1
+
 				}
 			}
-			runErr := status.FromProto(r.RunError)
-			err = fmt.Errorf("execution failed, %w", runErr.Err())
+			if !userError {
+				runErr := status.FromProto(r.RunError)
+				err = runErr.Err()
+				cl.UI().Output("Unexpected Error: "+err.Error()+"\n", terminal.WithErrorStyle())
+			}
 		}
 
 		c.Log.Debug("result from operation", "task", c.name, "result", r)
@@ -117,9 +128,11 @@ func (c *DynamicCommand) Run(args []string) int {
 	})
 
 	if err != nil {
+		c.Log.Error("Got error from task, so exiting 255", "error", err)
 		return int(-1)
 	}
 
+	c.Log.Info("Task did not error, so exiting with provided code", "code", r.ExitCode)
 	return int(r.ExitCode)
 }
 

plugins/commands/serve/util/exception_transformer.rb

@@ -1,3 +1,4 @@
+require 'google/protobuf/well_known_types'
 require 'google/rpc/error_details_pb'
 
 module VagrantPlugins
@@ -5,8 +6,6 @@ module VagrantPlugins
     module Util
       # Adds exception logging to all public instance methods
       module ExceptionTransformer
-        prepend Util::HasMapper
-
         def self.included(klass)
           # Get all the public instance methods. Need to search ancestors as well
           # for modules like the Guest service which includes the CapabilityPlatform
@@ -52,25 +51,25 @@ module VagrantPlugins
                 # headers below the limit in most cases.
                 message = ExceptionTransformer.truncate_to(err.message, 1024)
                 backtrace = ExceptionTransformer.truncate_to(err.backtrace.join("\n"), 1024)
-                localized_msg_details_any = Google::Protobuf::Any.new
-                localized_msg_details_any.pack(
-                  # This is the message we expect to be unpacked and presented
-                  # to the user, as it's sometimes a user-facing error.
-                  Google::Rpc::LocalizedMessage.new(
-                    locale: "en-US", message: message
+                metadata = {}
+
+                # VagrantErrors are user-facing and so get their message packed
+                # into the details.
+                if err.is_a? Vagrant::Errors::VagrantError
+                  localized_msg_details_any = Google::Protobuf::Any.new
+                  localized_msg_details_any.pack(
+                    Google::Rpc::LocalizedMessage.new(locale: "en-US", message: message)
                   )
-                )
-                proto = Google::Rpc::Status.new(
-                  code: GRPC::Core::StatusCodes::UNKNOWN,
-                  message: "#{message}\n#{backtrace}",
-                  details: [localized_msg_details_any]
-                )
-                encoded_proto = Google::Rpc::Status.encode(proto)
-                grpc_status_details_bin_trailer = 'grpc-status-details-bin'
+                  proto = Google::Rpc::Status.new(
+                    code: GRPC::Core::StatusCodes::UNKNOWN,
+                    details: [localized_msg_details_any]
+                  )
+                  metadata[GRPC_DETAILS_METADATA_KEY] = Google::Rpc::Status.encode(proto)
+                end
                 grpc_error = GRPC::BadStatus.new(
                   GRPC::Core::StatusCodes::UNKNOWN,
-                  message,
-                  {grpc_status_details_bin_trailer => encoded_proto},
+                  "#{message}\n#{backtrace}",
+                  metadata,
                 )
                 raise grpc_error
               end
@@ -87,6 +86,8 @@ module VagrantPlugins
             str[0, len-3] + "..."
           end
         end
+
+        GRPC_DETAILS_METADATA_KEY = "grpc-status-details-bin".freeze
       end
     end
   end

test/unit/plugins/commands/serve/util/exception_transformer_test.rb

@@ -0,0 +1,38 @@
+require File.expand_path("../../../../../base", __FILE__)
+
+require Vagrant.source_root.join("plugins/commands/serve/command")
+
+describe VagrantPlugins::CommandServe::Util::ExceptionTransformer do
+  include_context "unit"
+
+  it "converts VagrantErrors into GRPC::BadStatus errors with a LocalizedMessage" do
+    klass = Class.new
+    vagrant_error = Class.new(Vagrant::Errors::VagrantError) do
+      error_key("test_key")
+    end
+    klass.define_method(:wrapme) do |*args|
+      raise vagrant_error.new
+    end
+    subklass = Class.new(klass)
+    subklass.include described_class
+    expect {
+      subklass.new.wrapme
+    }.to raise_error(an_instance_of(GRPC::BadStatus).and satisfy { |err|
+      err.metadata["grpc-status-details-bin"] =~ /LocalizedMessage/
+    })
+  end
+
+  it "converts non-VagrantErrors into GRPC::BadStatus errors without a LocalizedMessage" do
+    klass = Class.new
+    klass.define_method(:wrapme) do
+      raise "just a regular error"
+    end
+    subklass = Class.new(klass)
+    subklass.include described_class
+    expect {
+      subklass.new.wrapme
+    }.to raise_error(an_instance_of(GRPC::BadStatus).and satisfy { |err|
+      err.metadata["grpc-status-details-bin"].nil?
+    })
+  end
+end