From da508d6a8b2cd0d0334f820c8605175106ca7c7f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Nov 2016 12:04:04 -0700 Subject: [PATCH 1/2] terraform: GraphWalkerPanicwrap catches panics during graph walks --- terraform/graph.go | 30 ++++++++++++++++++++++++++++++ terraform/graph_test.go | 23 +++++++++++++++++++++++ terraform/graph_walk.go | 32 +++++++++++++++++++++++++++++++- 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/terraform/graph.go b/terraform/graph.go index db492084ef6d..46762de5d8de 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "log" + "runtime/debug" "strings" "sync" @@ -217,11 +218,40 @@ func (g *Graph) walk(walker GraphWalker) error { // Get the path for logs path := strings.Join(ctx.Path(), ".") + // Determine if our walker is a panic wrapper + panicwrap, ok := walker.(GraphWalkerPanicwrapper) + if !ok { + panicwrap = nil // just to be sure + } + // Walk the graph. var walkFn dag.WalkFunc walkFn = func(v dag.Vertex) (rerr error) { log.Printf("[DEBUG] vertex '%s.%s': walking", path, dag.VertexName(v)) + // If we have a panic wrap GraphWalker and a panic occurs, recover + // and call that. We ensure the return value is an error, however, + // so that future nodes are not called. + defer func() { + // If no panicwrap, do nothing + if panicwrap == nil { + return + } + + // If no panic, do nothing + err := recover() + if err == nil { + return + } + + // Modify the return value to show the error + rerr = fmt.Errorf("vertex %q captured panic: %s\n\n%s", + dag.VertexName(v), err, debug.Stack()) + + // Call the panic wrapper + panicwrap.Panic(v, err) + }() + walker.EnterVertex(v) defer func() { walker.ExitVertex(v, rerr) }() diff --git a/terraform/graph_test.go b/terraform/graph_test.go index ef91fec4acd4..45770e2136f3 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -69,6 +69,29 @@ func TestGraphReplace_DependableWithNonDependable(t *testing.T) { } } +func TestGraphWalk_panicWrap(t *testing.T) { + var g Graph + + // Add our crasher + v := &testGraphSubPath{ + PathFn: func() []string { + panic("yo") + }, + } + g.Add(v) + + err := g.Walk(GraphWalkerPanicwrap(new(NullGraphWalker))) + if err == nil { + t.Fatal("should error") + } +} + +type testGraphSubPath struct { + PathFn func() []string +} + +func (v *testGraphSubPath) Path() []string { return v.PathFn() } + type testGraphDependable struct { VertexName string DependentOnMock []string diff --git a/terraform/graph_walk.go b/terraform/graph_walk.go index ef3a4f6f5477..34ce6f6404c1 100644 --- a/terraform/graph_walk.go +++ b/terraform/graph_walk.go @@ -15,12 +15,42 @@ type GraphWalker interface { ExitEvalTree(dag.Vertex, interface{}, error) error } +// GrpahWalkerPanicwrapper can be optionally implemented to catch panics +// that occur while walking the graph. This is not generally recommended +// since panics should crash Terraform and result in a bug report. However, +// this is particularly useful for situations like the shadow graph where +// you don't ever want to cause a panic. +type GraphWalkerPanicwrapper interface { + GraphWalker + + // Panic is called when a panic occurs. This will halt the panic from + // propogating so if the walker wants it to crash still it should panic + // again. This is called from within a defer so runtime/debug.Stack can + // be used to get the stack trace of the panic. + Panic(dag.Vertex, interface{}) +} + +// GraphWalkerPanicwrap wraps an existing Graphwalker to wrap and swallow +// the panics. This doesn't lose the panics since the panics are still +// returned as errors as part of a graph walk. +func GraphWalkerPanicwrap(w GraphWalker) GraphWalkerPanicwrapper { + return &graphWalkerPanicwrapper{ + GraphWalker: w, + } +} + +type graphWalkerPanicwrapper struct { + GraphWalker +} + +func (graphWalkerPanicwrapper) Panic(dag.Vertex, interface{}) {} + // NullGraphWalker is a GraphWalker implementation that does nothing. // This can be embedded within other GraphWalker implementations for easily // implementing all the required functions. type NullGraphWalker struct{} -func (NullGraphWalker) EnterPath([]string) EvalContext { return nil } +func (NullGraphWalker) EnterPath([]string) EvalContext { return new(MockEvalContext) } func (NullGraphWalker) ExitPath([]string) {} func (NullGraphWalker) EnterVertex(dag.Vertex) {} func (NullGraphWalker) ExitVertex(dag.Vertex, error) {} From 90bfff30269f84bfce5db1236fba158a105d0cba Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Nov 2016 12:09:51 -0700 Subject: [PATCH 2/2] terraform: shadow graph uses GraphWalkerPanicwrap to catch errors --- terraform/context.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 80bdf2d5096d..c8aac252d00e 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -736,11 +736,14 @@ func (c *Context) walk( // If we have a shadow graph, wait for that to complete. if shadowCloser != nil { - // Build the graph walker for the shadow. - shadowWalker := &ContextGraphWalker{ + // Build the graph walker for the shadow. We also wrap this in + // a panicwrap so that panics are captured. For the shadow graph, + // we just want panics to be normal errors rather than to crash + // Terraform. + shadowWalker := GraphWalkerPanicwrap(&ContextGraphWalker{ Context: shadowCtx, Operation: operation, - } + }) // Kick off the shadow walk. This will block on any operations // on the real walk so it is fine to start first.