packer, packer/rpc: Make command/builderFunc support errors

packer/environment.go

@@ -9,9 +9,9 @@ import (
 	"strings"
 )
 
-type BuilderFunc func(name string) Builder
+type BuilderFunc func(name string) (Builder, error)
 
-type CommandFunc func(name string) Command
+type CommandFunc func(name string) (Command, error)
 
 // The environment interface provides access to the configuration and
 // state of a single Packer run.
@@ -19,8 +19,8 @@ type CommandFunc func(name string) Command
 // It allows for things such as executing CLI commands, getting the
 // list of available builders, and more.
 type Environment interface {
-	Builder(name string) Builder
-	Cli(args []string) int
+	Builder(name string) (Builder, error)
+	Cli(args []string) (int, error)
 	Ui() Ui
 }
 
@@ -45,8 +45,8 @@ type EnvironmentConfig struct {
 // be used to create a new enviroment with NewEnvironment with sane defaults.
 func DefaultEnvironmentConfig() *EnvironmentConfig {
 	config := &EnvironmentConfig{}
-	config.BuilderFunc = func(string) Builder { return nil }
-	config.CommandFunc = func(string) Command { return nil }
+	config.BuilderFunc = func(string) (Builder, error) { return nil, nil }
+	config.CommandFunc = func(string) (Command, error) { return nil, nil }
 	config.Commands = make([]string, 0)
 	config.Ui = &ReaderWriterUi{os.Stdin, os.Stdout}
 	return config
@@ -71,16 +71,25 @@ func NewEnvironment(config *EnvironmentConfig) (resultEnv Environment, err error
 
 // Returns a builder of the given name that is registered with this
 // environment.
-func (e *coreEnvironment) Builder(name string) Builder {
-	return e.builderFunc(name)
+func (e *coreEnvironment) Builder(name string) (b Builder, err error) {
+	b, err = e.builderFunc(name)
+	if err != nil {
+		return
+	}
+
+	if b == nil {
+		err = fmt.Errorf("No builder returned for name: %s", name)
+	}
+
+	return
 }
 
 // Executes a command as if it was typed on the command-line interface.
 // The return value is the exit code of the command.
-func (e *coreEnvironment) Cli(args []string) int {
+func (e *coreEnvironment) Cli(args []string) (result int, err error) {
 	if len(args) == 0 || args[0] == "--help" || args[0] == "-h" {
 		e.printHelp()
-		return 1
+		return 1, nil
 	}
 
 	version := args[0] == "version"
@@ -99,16 +108,19 @@ func (e *coreEnvironment) Cli(args []string) int {
 	}
 
 	if command == nil {
-		command = e.commandFunc(args[0])
+		command, err = e.commandFunc(args[0])
+		if err != nil {
+			return
+		}
 
 		// If we still don't have a command, show the help.
 		if command == nil {
 			e.printHelp()
-			return 1
+			return 1, nil
 		}
 	}
 
-	return command.Run(e, args[1:])
+	return command.Run(e, args[1:]), nil
 }
 
 // Prints the CLI help to the UI.
@@ -131,13 +143,20 @@ func (e *coreEnvironment) printHelp() {
 	e.ui.Say("usage: packer [--version] [--help] <command> [<args>]\n\n")
 	e.ui.Say("Available commands are:\n")
 	for _, key := range e.commands {
-		command := e.commandFunc(key)
+		var synopsis string
+
+		command, err := e.commandFunc(key)
+		if err != nil {
+			synopsis = fmt.Sprintf("Error loading command: %s", err.Error())
+		} else {
+			synopsis = command.Synopsis()
+		}
 
 		// Pad the key with spaces so that they're all the same width
 		key = fmt.Sprintf("%v%v", key, strings.Repeat(" ", maxKeyLen-len(key)))
 
 		// Output the command and the synopsis
-		e.ui.Say("    %v     %v\n", key, command.Synopsis())
+		e.ui.Say("    %v     %v\n", key, synopsis)
 	}
 }
 

packer/environment_test.go

@@ -3,6 +3,7 @@ package packer
 import (
 	"bytes"
 	"cgl.tideland.biz/asserts"
+	"fmt"
 	"os"
 	"strings"
 	"testing"
@@ -58,10 +59,12 @@ func TestEnvironment_Builder(t *testing.T) {
 	builders["foo"] = builder
 
 	config := DefaultEnvironmentConfig()
-	config.BuilderFunc = func(n string) Builder { return builders[n] }
+	config.BuilderFunc = func(n string) (Builder, error) { return builders[n], nil }
 
 	env, _ := NewEnvironment(config)
-	assert.Equal(env.Builder("foo"), builder, "should return correct builder")
+	returnedBuilder, err := env.Builder("foo")
+	assert.Nil(err, "should be no error")
+	assert.Equal(returnedBuilder, builder, "should return correct builder")
 }
 
 func TestEnvironment_Cli_CallsRun(t *testing.T) {
@@ -73,10 +76,12 @@ func TestEnvironment_Cli_CallsRun(t *testing.T) {
 
 	config := &EnvironmentConfig{}
 	config.Commands = []string{"foo"}
-	config.CommandFunc = func(n string) Command { return commands[n] }
+	config.CommandFunc = func(n string) (Command, error) { return commands[n], nil }
 
 	env, _ := NewEnvironment(config)
-	assert.Equal(env.Cli([]string{"foo", "bar", "baz"}), 0, "runs foo command")
+	exitCode, err := env.Cli([]string{"foo", "bar", "baz"})
+	assert.Nil(err, "should be no error")
+	assert.Equal(exitCode, 0, "runs foo command")
 	assert.True(command.runCalled, "run should've been called")
 	assert.Equal(command.runEnv, env, "should've ran with env")
 	assert.Equal(command.runArgs, []string{"bar", "baz"}, "should have right args")
@@ -87,7 +92,8 @@ func TestEnvironment_DefaultCli_Empty(t *testing.T) {
 
 	defaultEnv := testEnvironment()
 
-	assert.Equal(defaultEnv.Cli([]string{}), 1, "CLI with no args")
+	exitCode, _ := defaultEnv.Cli([]string{})
+	assert.Equal(exitCode, 1, "CLI with no args")
 }
 
 func TestEnvironment_DefaultCli_Help(t *testing.T) {
@@ -104,11 +110,13 @@ func TestEnvironment_DefaultCli_Help(t *testing.T) {
 	}
 
 	// Test "--help"
-	assert.Equal(defaultEnv.Cli([]string{"--help"}), 1, "--help should print")
+	exitCode, _ := defaultEnv.Cli([]string{"--help"})
+	assert.Equal(exitCode, 1, "--help should print")
 	testOutput()
 
 	// Test "-h"
-	assert.Equal(defaultEnv.Cli([]string{"-h"}), 1, "--help should print")
+	exitCode, _ = defaultEnv.Cli([]string{"--help"})
+	assert.Equal(exitCode, 1, "--help should print")
 	testOutput()
 }
 
@@ -117,17 +125,20 @@ func TestEnvironment_DefaultCli_Version(t *testing.T) {
 
 	defaultEnv := testEnvironment()
 
-	// Test the basic version options
-	assert.Equal(defaultEnv.Cli([]string{"version"}), 0, "version should work")
-	assert.Equal(defaultEnv.Cli([]string{"--version"}), 0, "--version should work")
-	assert.Equal(defaultEnv.Cli([]string{"-v"}), 0, "-v should work")
+	versionCommands := []string{"version", "--version", "-v"}
+	for _, command := range versionCommands {
+		exitCode, _ := defaultEnv.Cli([]string{command})
+		assert.Equal(exitCode, 0, fmt.Sprintf("%s should work", command))
 
 		// Test the --version and -v can appear anywhere
-	assert.Equal(defaultEnv.Cli([]string{"bad", "-v"}), 0, "-v should work anywhere")
-	assert.Equal(defaultEnv.Cli([]string{"bad", "--version"}), 0, "--version should work anywhere")
+		exitCode, _ = defaultEnv.Cli([]string{"bad", command})
 
-	// Test that "version" can't appear anywhere
-	assert.Equal(defaultEnv.Cli([]string{"bad", "version"}), 1, "version should NOT work anywhere")
+		if command != "version" {
+			assert.Equal(exitCode, 0, fmt.Sprintf("%s should work anywhere", command))
+		} else {
+			assert.Equal(exitCode, 1, fmt.Sprintf("%s should NOT work anywhere", command))
+		}
+	}
 }
 
 func TestEnvironment_SettingUi(t *testing.T) {

packer/rpc/environment.go

@@ -21,18 +21,25 @@ type EnvironmentCliArgs struct {
 	Args []string
 }
 
-func (e *Environment) Builder(name string) packer.Builder {
+func (e *Environment) Builder(name string) (b packer.Builder, err error) {
 	var reply string
-	e.client.Call("Environment.Builder", name, &reply)
+	err = e.client.Call("Environment.Builder", name, &reply)
+	if err != nil {
+		return
+	}
 
-	// TODO: error handling
-	client, _ := rpc.Dial("tcp", reply)
-	return &Builder{client}
+	client, err := rpc.Dial("tcp", reply)
+	if err != nil {
+		return
+	}
+
+	b = &Builder{client}
+	return
 }
 
-func (e *Environment) Cli(args []string) (result int) {
+func (e *Environment) Cli(args []string) (result int, err error) {
 	rpcArgs := &EnvironmentCliArgs{args}
-	e.client.Call("Environment.Cli", rpcArgs, &result)
+	err = e.client.Call("Environment.Cli", rpcArgs, &result)
 	return
 }
 
@@ -46,7 +53,10 @@ func (e *Environment) Ui() packer.Ui {
 }
 
 func (e *EnvironmentServer) Builder(name *string, reply *string) error {
-	builder := e.env.Builder(*name)
+	builder, err := e.env.Builder(*name)
+	if err != nil {
+		return err
+	}
 
 	// Wrap it
 	server := rpc.NewServer()
@@ -56,9 +66,9 @@ func (e *EnvironmentServer) Builder(name *string, reply *string) error {
 	return nil
 }
 
-func (e *EnvironmentServer) Cli(args *EnvironmentCliArgs, reply *int) error {
-	*reply = e.env.Cli(args.Args)
-	return nil
+func (e *EnvironmentServer) Cli(args *EnvironmentCliArgs, reply *int) (err error) {
+	*reply, err = e.env.Cli(args.Args)
+	return
 }
 
 func (e *EnvironmentServer) Ui(args *interface{}, reply *string) error {

packer/rpc/environment_test.go

@@ -18,16 +18,16 @@ type testEnvironment struct {
 	uiCalled bool
 }
 
-func (e *testEnvironment) Builder(name string) packer.Builder {
+func (e *testEnvironment) Builder(name string) (packer.Builder, error) {
 	e.builderCalled = true
 	e.builderName = name
-	return testEnvBuilder
+	return testEnvBuilder, nil
 }
 
-func (e *testEnvironment) Cli(args []string) int {
+func (e *testEnvironment) Cli(args []string) (int, error) {
 	e.cliCalled = true
 	e.cliArgs = args
-	return 42
+	return 42, nil
 }
 
 func (e *testEnvironment) Ui() packer.Ui {
@@ -52,7 +52,7 @@ func TestEnvironmentRPC(t *testing.T) {
 	eClient := &Environment{client}
 
 	// Test Builder
-	builder := eClient.Builder("foo")
+	builder, _ := eClient.Builder("foo")
 	assert.True(e.builderCalled, "Builder should be called")
 	assert.Equal(e.builderName, "foo", "Correct name for Builder")
 
@@ -61,7 +61,7 @@ func TestEnvironmentRPC(t *testing.T) {
 
 	// Test Cli
 	cliArgs := []string{"foo", "bar"}
-	result := eClient.Cli(cliArgs)
+	result, _ := eClient.Cli(cliArgs)
 	assert.True(e.cliCalled, "CLI should be called")
 	assert.Equal(e.cliArgs, cliArgs, "args should match")
 	assert.Equal(result, 42, "result shuld be 42")

packer/template.go

@@ -102,9 +102,13 @@ func (t *Template) Build(name string, bf BuilderFunc) (b Build, err error) {
 		return
 	}
 
-	builder := bf(builderConfig.builderName)
+	builder, err := bf(builderConfig.builderName)
+	if err != nil {
+		return
+	}
+
 	if builder == nil {
-		err = fmt.Errorf("Builder could not be found: %s", builderConfig.builderName)
+		err = fmt.Errorf("Builder not found: %s", name)
 		return
 	}
 

packer/template_test.go

@@ -157,7 +157,7 @@ func TestTemplate_BuildUnknownBuilder(t *testing.T) {
 	template, err := ParseTemplate([]byte(data))
 	assert.Nil(err, "should not error")
 
-	builderFactory := func(string) Builder { return nil }
+	builderFactory := func(string) (Builder, error) { return nil, nil }
 	build, err := template.Build("test1", builderFactory)
 	assert.Nil(build, "build should be nil")
 	assert.NotNil(err, "should have error")
@@ -191,7 +191,7 @@ func TestTemplate_Build(t *testing.T) {
 		"test-builder": builder,
 	}
 
-	builderFactory := func(n string) Builder { return builderMap[n] }
+	builderFactory := func(n string) (Builder, error) { return builderMap[n], nil }
 
 	// Get the build, verifying we can get it without issue, but also
 	// that the proper builder was looked up and used for the build.