depgraph: if a walk func errors, don't walk dependents

depgraph/graph.go

@@ -259,6 +259,10 @@ func (g *Graph) Walk(fn WalkFunc) error {
 	seenMap := make(map[*Noun]chan struct{})
 	seenMap[g.Root] = make(chan struct{})
 
+	// Keep track of what nodes errored.
+	var errMapL sync.RWMutex
+	errMap := make(map[*Noun]struct{})
+
 	// Build the list of things to visit
 	tovisit := make([]*Noun, 1, len(g.Nouns))
 	tovisit[0] = g.Root
@@ -302,10 +306,23 @@ func (g *Graph) Walk(fn WalkFunc) error {
 				case <-quitCh:
 					return
 				}
+
+				// Check if any dependencies errored. If so,
+				// then return right away, we won't walk it.
+				errMapL.RLock()
+				_, errOk := errMap[dep.Target]
+				errMapL.RUnlock()
+				if errOk {
+					return
+				}
 			}
 
 			// Call our callback!
 			if err := fn(current); err != nil {
+				errMapL.Lock()
+				errMap[current] = struct{}{}
+				errMapL.Unlock()
+
 				errCh <- err
 			}
 		}(current)

depgraph/graph_test.go

@@ -388,21 +388,40 @@ c -> e`)
 
 func TestGraphWalk_error(t *testing.T) {
 	nodes := ParseNouns(`a -> b
-a -> c
-b -> d
-b -> e
-c -> d
-c -> e`)
+b -> c
+a -> d`)
 	list := NounMapToList(nodes)
 	g := &Graph{Name: "Test", Nouns: list}
 	if err := g.Validate(); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
-	err := g.Walk(func(n *Noun) error {
-		return fmt.Errorf("foo")
-	})
-	if err == nil {
-		t.Fatal("should error")
+	// We repeat this a lot because sometimes timing causes
+	// a false positive.
+	for i := 0; i < 100; i++ {
+		var lock sync.Mutex
+		var walked []string
+		err := g.Walk(func(n *Noun) error {
+			lock.Lock()
+			defer lock.Unlock()
+
+			walked = append(walked, n.Name)
+
+			if n.Name == "b" {
+				return fmt.Errorf("foo")
+			}
+
+			return nil
+		})
+		if err == nil {
+			t.Fatal("should error")
+		}
+
+		sort.Strings(walked)
+
+		expected := []string{"b", "c", "d"}
+		if !reflect.DeepEqual(walked, expected) {
+			t.Fatalf("bad: %#v", walked)
+		}
 	}
 }