Fragment.Close now returns an error.

This required changing the Holder test utility Reopen, since in those cases we needed to close the file before making permission changes that Close() was failing silently on.

fragment.go

@@ -293,11 +293,13 @@ func (f *Fragment) close() error {
 	// Flush cache if closing gracefully.
 	if err := f.flushCache(); err != nil {
 		f.logger().Printf("fragment: error flushing cache on close: err=%s, path=%s", err, f.path)
+		return err
 	}
 
 	// Close underlying storage.
 	if err := f.closeStorage(); err != nil {
 		f.logger().Printf("fragment: error closing storage: err=%s, path=%s", err, f.path)
+		return err
 	}
 
 	// Remove checksums.

frame.go

@@ -401,7 +401,9 @@ func (f *Frame) Close() error {
 
 	// Close all views.
 	for _, view := range f.views {
-		_ = view.Close()
+		if err := view.Close(); err != nil {
+			return err
+		}
 	}
 	f.views = make(map[string]*View)
 
@@ -537,8 +539,10 @@ func (f *Frame) deleteView(name string) error {
 		return ErrInvalidView
 	}
 
-	// TODO capture errors lower down in this method
-	_ = view.Close()
+	// Close data files before deletion
+	if err := view.Close(); err != nil {
+		return err
+	}
 
 	// Delete view directory.
 	if err := os.RemoveAll(view.Path()); err != nil {

holder.go

@@ -133,7 +133,9 @@ func (h *Holder) Close() error {
 	h.wg.Wait()
 
 	for _, index := range h.indexes {
-		index.Close()
+		if err := index.Close(); err != nil {
+			return err
+		}
 	}
 	return nil
 }

holder_test.go

@@ -34,8 +34,9 @@ func TestHolder_Open(t *testing.T) {
 
 		if err := os.Mkdir(h.IndexPath("!"), 0777); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		}
-
 		if err := h.Reopen(); err != nil {
 			t.Fatal(err)
 		} else if logOutput := h.LogOutput.String(); !strings.Contains(logOutput, `ERROR opening index: !`) {
@@ -49,6 +50,8 @@ func TestHolder_Open(t *testing.T) {
 
 		if _, err := h.CreateIndex("test", pilosa.IndexOptions{}); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Chmod(h.IndexPath("test"), 0000); err != nil {
 			t.Fatal(err)
 		}
@@ -64,6 +67,8 @@ func TestHolder_Open(t *testing.T) {
 
 		if _, err := h.CreateIndex("test", pilosa.IndexOptions{}); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Truncate(filepath.Join(h.IndexPath("test"), ".meta"), 2); err != nil {
 			t.Fatal(err)
 		}
@@ -78,6 +83,8 @@ func TestHolder_Open(t *testing.T) {
 
 		if _, err := h.CreateIndex("test", pilosa.IndexOptions{}); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Truncate(filepath.Join(h.IndexPath("test"), ".data"), 2); err != nil {
 			t.Fatal(err)
 		}
@@ -95,6 +102,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if _, err := idx.CreateFrame("bar", pilosa.FrameOptions{}); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar"), 0000); err != nil {
 			t.Fatal(err)
 		}
@@ -112,6 +121,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if _, err := idx.CreateFrame("bar", pilosa.FrameOptions{}); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Truncate(filepath.Join(h.Path, "foo", "bar", ".meta"), 2); err != nil {
 			t.Fatal(err)
 		}
@@ -128,6 +139,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if _, err := idx.CreateFrame("bar", pilosa.FrameOptions{}); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Truncate(filepath.Join(h.Path, "foo", "bar", ".data"), 2); err != nil {
 			t.Fatal(err)
 		}
@@ -147,6 +160,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if _, err := frame.CreateViewIfNotExists(pilosa.ViewStandard); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard"), 0000); err != nil {
 			t.Fatal(err)
 		}
@@ -166,6 +181,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if _, err := frame.CreateViewIfNotExists(pilosa.ViewStandard); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments"), 0000); err != nil {
 			t.Fatal(err)
 		}
@@ -188,6 +205,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if _, err := view.SetBit(0, 0); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0"), 0000); err != nil {
 			t.Fatal(err)
 		}
@@ -209,6 +228,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if _, err := view.SetBit(0, 0); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Truncate(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0"), 2); err != nil {
 			t.Fatal(err)
 		}
@@ -232,6 +253,8 @@ func TestHolder_Open(t *testing.T) {
 			t.Fatal(err)
 		} else if err := view.Fragment(0).FlushCache(); err != nil {
 			t.Fatal(err)
+		} else if err := h.Holder.Close(); err != nil {
+			t.Fatal(err)
 		} else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0.cache"), 0000); err != nil {
 			t.Fatal(err)
 		}

index.go

@@ -248,7 +248,9 @@ func (i *Index) Close() error {
 
 	// Close all frames.
 	for _, f := range i.frames {
-		f.Close()
+		if err := f.Close(); err != nil {
+			return err
+		}
 	}
 	i.frames = make(map[string]*Frame)
 

test/holder.go

@@ -45,9 +45,9 @@ func (h *Holder) Close() error {
 
 // Reopen closes the holder and instantiates and opens a new holder.
 func (h *Holder) Reopen() error {
-	if err := h.Holder.Close(); err != nil {
-		return err
-	}
+	// if err := h.Holder.Close(); err != nil {
+	// 	return err
+	// }
 
 	path, logOutput := h.Path, h.Holder.LogOutput
 	h.Holder = pilosa.NewHolder()

view.go

@@ -162,7 +162,9 @@ func (v *View) Close() error {
 
 	// Close all fragments.
 	for _, frag := range v.fragments {
-		_ = frag.Close()
+		if err := frag.Close(); err != nil {
+			return err
+		}
 	}
 	v.fragments = make(map[uint64]*Fragment)