From 73a1be95feba9c1b83b39b2c6b95e4e51a7b5e64 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 30 Mar 2022 15:24:43 -0500 Subject: [PATCH] Fix nil dereference bug in new error handling logic Found this while running `./vagrant box` with no args to get the help output. It turns out you can have an empty RunResult but also a nil error. I took the occasion to unwind the conditional tree a bit which hopefully makes it a bit easier to read. --- internal/cli/dynamic.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/cli/dynamic.go b/internal/cli/dynamic.go index e2f85831e..116ecd26d 100644 --- a/internal/cli/dynamic.go +++ b/internal/cli/dynamic.go @@ -95,10 +95,9 @@ func (c *DynamicCommand) Run(args []string) int { modifier, ) - if err != nil { - cl.UI().Output("Running of task "+c.name+" failed unexpectedly\n", terminal.WithErrorStyle()) - cl.UI().Output("Error: "+err.Error(), terminal.WithErrorStyle()) - } else if !r.RunResult { + // If nothing failed but we didn't get a Result back, something may + // have gone wrong on the far side so we need to interpret the error. + if err == nil && !r.RunResult { runErrorStatus := status.FromProto(r.RunError) details := runErrorStatus.Details() userError := false @@ -112,16 +111,21 @@ func (c *DynamicCommand) Run(args []string) int { // All user-facing errors from Ruby use a 1 exit code. See // Vagrant::Errors::VagrantError. r.ExitCode = 1 - } } + // If there wasn't a user-facing error, just assign the returned + // error (if any) from the response and assign that back out so it + // can be displayed as an unexpected error. if !userError { - runErr := status.FromProto(r.RunError) - err = runErr.Err() - cl.UI().Output("Unexpected Error: "+err.Error()+"\n", terminal.WithErrorStyle()) + err = runErrorStatus.Err() } } + if err != nil { + cl.UI().Output("Running of task "+c.name+" failed unexpectedly\n", terminal.WithErrorStyle()) + cl.UI().Output("Error: "+err.Error(), terminal.WithErrorStyle()) + } + c.Log.Debug("result from operation", "task", c.name, "result", r) return err