packer, packer/rpc: Build/builders can return errors now

packer/build.go

@@ -7,7 +7,7 @@ import "log"
 type Build interface {
 	Name() string
 	Prepare(Ui) error
-	Run(Ui, Cache) Artifact
+	Run(Ui, Cache) (Artifact, error)
 	Cancel()
 }
 
@@ -61,7 +61,7 @@ func (b *coreBuild) Prepare(ui Ui) (err error) {
 }
 
 // Runs the actual build. Prepare must be called prior to running this.
-func (b *coreBuild) Run(ui Ui, cache Cache) Artifact {
+func (b *coreBuild) Run(ui Ui, cache Cache) (Artifact, error) {
 	if !b.prepareCalled {
 		panic("Prepare must be called first")
 	}

packer/builder.go

@@ -10,7 +10,7 @@ type Builder interface {
 	Prepare(config interface{}) error
 
 	// Run is where the actual build should take place. It takes a Build and a Ui.
-	Run(ui Ui, hook Hook, cache Cache) Artifact
+	Run(ui Ui, hook Hook, cache Cache) (Artifact, error)
 
 	// Cancel cancels a possibly running Builder. This should block until
 	// the builder actually cancels and cleans up after itself.

packer/builder_test.go

@@ -16,12 +16,12 @@ func (tb *TestBuilder) Prepare(config interface{}) error {
 	return nil
 }
 
-func (tb *TestBuilder) Run(ui Ui, h Hook, c Cache) Artifact {
+func (tb *TestBuilder) Run(ui Ui, h Hook, c Cache) (Artifact, error) {
 	tb.runCalled = true
 	tb.runHook = h
 	tb.runUi = ui
 	tb.runCache = c
-	return nil
+	return nil, nil
 }
 
 func (tb *TestBuilder) Cancel() {

packer/rpc/build.go

@@ -48,7 +48,7 @@ func (b *build) Prepare(ui packer.Ui) (err error) {
 	return
 }
 
-func (b *build) Run(ui packer.Ui, cache packer.Cache) packer.Artifact {
+func (b *build) Run(ui packer.Ui, cache packer.Cache) (packer.Artifact, error) {
 	// Create and start the server for the UI
 	server := rpc.NewServer()
 	RegisterCache(server, cache)
@@ -65,7 +65,7 @@ func (b *build) Run(ui packer.Ui, cache packer.Cache) packer.Artifact {
 		panic(err)
 	}
 
-	return Artifact(client)
+	return Artifact(client), nil
 }
 
 func (b *build) Cancel() {
@@ -95,7 +95,10 @@ func (b *BuildServer) Run(args *BuildRunArgs, reply *string) error {
 		return err
 	}
 
-	artifact := b.build.Run(&Ui{client}, Cache(client))
+	artifact, err := b.build.Run(&Ui{client}, Cache(client))
+	if err != nil {
+		return err
+	}
 
 	// Wrap the artifact
 	server := rpc.NewServer()

packer/rpc/build_test.go

@@ -30,11 +30,11 @@ func (b *testBuild) Prepare(ui packer.Ui) error {
 	return nil
 }
 
-func (b *testBuild) Run(ui packer.Ui, cache packer.Cache) packer.Artifact {
+func (b *testBuild) Run(ui packer.Ui, cache packer.Cache) (packer.Artifact, error) {
 	b.runCalled = true
 	b.runCache = cache
 	b.runUi = ui
-	return testBuildArtifact
+	return testBuildArtifact, nil
 }
 
 func (b *testBuild) Cancel() {

packer/rpc/builder.go

@@ -30,6 +30,7 @@ type BuilderRunArgs struct {
 }
 
 type BuilderRunResponse struct {
+	Err        error
 	RPCAddress string
 }
 
@@ -46,7 +47,7 @@ func (b *builder) Prepare(config interface{}) (err error) {
 	return
 }
 
-func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer.Artifact {
+func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {
 	// Create and start the server for the Build and UI
 	server := rpc.NewServer()
 	RegisterCache(server, cache)
@@ -55,7 +56,7 @@ func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer
 
 	// Create a server for the response
 	responseL := netListenerInRange(portRangeMin, portRangeMax)
-	artifactAddress := make(chan string)
+	runResponseCh := make(chan *BuilderRunResponse)
 	go func() {
 		defer responseL.Close()
 
@@ -72,7 +73,7 @@ func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer
 			log.Panic(err)
 		}
 
-		artifactAddress <- response.RPCAddress
+		runResponseCh <- &response
 	}()
 
 	args := &BuilderRunArgs{
@@ -81,20 +82,24 @@ func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer
 	}
 
 	if err := b.client.Call("Builder.Run", args, new(interface{})); err != nil {
-		panic(err)
+		return nil, err
+	}
+
+	response := <-runResponseCh
+	if response.Err != nil {
+		return nil, response.Err
 	}
 
-	address := <-artifactAddress
-	if address == "" {
-		return nil
+	if response.RPCAddress == "" {
+		return nil, nil
 	}
 
-	client, err := rpc.Dial("tcp", address)
+	client, err := rpc.Dial("tcp", response.RPCAddress)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
 
-	return Artifact(client)
+	return Artifact(client), nil
 }
 
 func (b *builder) Cancel() {
@@ -132,17 +137,24 @@ func (b *BuilderServer) Run(args *BuilderRunArgs, reply *interface{}) error {
 		cache := Cache(client)
 		hook := Hook(client)
 		ui := &Ui{client}
-		artifact := b.builder.Run(ui, hook, cache)
+		artifact, responseErr := b.builder.Run(ui, hook, cache)
 		responseAddress := ""
 
-		if artifact != nil {
+		if responseErr == nil && artifact != nil {
 			// Wrap the artifact
 			server := rpc.NewServer()
 			RegisterArtifact(server, artifact)
 			responseAddress = serveSingleConn(server)
 		}
 
-		responseWriter.Encode(&BuilderRunResponse{responseAddress})
+		if responseErr != nil {
+			responseErr = NewBasicError(responseErr)
+		}
+
+		err := responseWriter.Encode(&BuilderRunResponse{responseErr, responseAddress})
+		if err != nil {
+			panic(err)
+		}
 	}()
 
 	return nil

packer/rpc/builder_test.go

@@ -2,6 +2,7 @@ package rpc
 
 import (
 	"cgl.tideland.biz/asserts"
+	"errors"
 	"github.com/mitchellh/packer/packer"
 	"net/rpc"
 	"testing"
@@ -18,6 +19,7 @@ type testBuilder struct {
 	runUi         packer.Ui
 	cancelCalled  bool
 
+	errRunResult bool
 	nilRunResult bool
 }
 
@@ -27,16 +29,18 @@ func (b *testBuilder) Prepare(config interface{}) error {
 	return nil
 }
 
-func (b *testBuilder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer.Artifact {
+func (b *testBuilder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {
 	b.runCache = cache
 	b.runCalled = true
 	b.runHook = hook
 	b.runUi = ui
 
-	if !b.nilRunResult {
-		return testBuilderArtifact
+	if b.errRunResult {
+		return nil, errors.New("foo")
+	} else if b.nilRunResult {
+		return nil, nil
 	} else {
-		return nil
+		return testBuilderArtifact, nil
 	}
 }
 
@@ -70,7 +74,8 @@ func TestBuilderRPC(t *testing.T) {
 	cache := new(testCache)
 	hook := &testHook{}
 	ui := &testUi{}
-	artifact := bClient.Run(ui, hook, cache)
+	artifact, err := bClient.Run(ui, hook, cache)
+	assert.Nil(err, "should have no error")
 	assert.True(b.runCalled, "runs hould be called")
 
 	if b.runCalled {
@@ -89,8 +94,16 @@ func TestBuilderRPC(t *testing.T) {
 
 	// Test run with nil result
 	b.nilRunResult = true
-	artifact = bClient.Run(ui, hook, cache)
+	artifact, err = bClient.Run(ui, hook, cache)
 	assert.Nil(artifact, "should be nil")
+	assert.Nil(err, "should have no error")
+
+	// Test with an error
+	b.errRunResult = true
+	b.nilRunResult = false
+	artifact, err = bClient.Run(ui, hook, cache)
+	assert.Nil(artifact, "should be nil")
+	assert.NotNil(err, "should have error")
 
 	// Test Cancel
 	bClient.Cancel()