diff --git a/internal/cli/dynamic.go b/internal/cli/dynamic.go index b472d90fd..e2f85831e 100644 --- a/internal/cli/dynamic.go +++ b/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) } diff --git a/plugins/commands/serve/util/exception_transformer.rb b/plugins/commands/serve/util/exception_transformer.rb index ee3ef6a05..d8c4779af 100644 --- a/plugins/commands/serve/util/exception_transformer.rb +++ b/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 diff --git a/test/unit/plugins/commands/serve/util/exception_transformer_test.rb b/test/unit/plugins/commands/serve/util/exception_transformer_test.rb new file mode 100644 index 000000000..da2121d7f --- /dev/null +++ b/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