3 files changed, 91 insertions(+), 12 deletions(-)

M web/handlers_repo.go
M web/templates/overview.html
M web/web_test.go

@@ 2,12 2,15 @@ package web
 
 import (
 	"errors"
+	"log/slog"
 	"net/http"
 	"strings"
 
 	"github.com/go-chi/chi/v5"
 	"sourcecraft.dev/bigbes/sr-ht-core/config"
 
+	"go.bigb.es/auxilia/scribe"
+
 	"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
 	"sourcecraft.dev/bigbes/sr-ht-ecore/pages"
 


@@ 193,12 196,25 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var (
-		branches  []browse.Branch
-		defBr     string
-		commits   []browse.CommitInfo
-		views     []View
-		browseErr string
+		branches []browse.Branch
+		defBr    string
+		commits  []browse.CommitInfo
+		views    []View
+		// browseFailed says only that the history could not be read. It used to
+		// be the browse layer's own error text, rendered into the page: that
+		// string names dolt internals and the store's on-disk path, which
+		// nothing else on this surface discloses and which a reader can do
+		// nothing with. A bool rather than a message, so no error can reach the
+		// template by being assigned to it later.
+		browseFailed bool
 	)
+	// browseFailure records the reason where it belongs — the operator's log,
+	// against the database it happened to — and leaves the page a fixed sentence.
+	browseFailure := func(what string, err error) {
+		browseFailed = true
+		slog.Warn(what, "component", "web", "database", repo.ID, scribe.Err(err))
+	}
+
 	if sess, err := a.cfg.Browse.Open(r.Context(), repo.Path); err == nil {
 		defer sess.Close()
 		if bs, err := sess.Branches(r.Context()); err == nil {


@@ 208,7 224,7 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) {
 				if cs, _, err := sess.Log(r.Context(), defBr, "", overviewCommitLimit); err == nil {
 					commits = cs
 				} else {
-					browseErr = err.Error()
+					browseFailure("reading a database's log for the overview failed", err)
 				}
 				// Fingerprint the tables at the default branch to compute the
 				// optional alternative-view tabs. A browse failure here must not


@@ 218,10 234,10 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) {
 				}
 			}
 		} else {
-			browseErr = err.Error()
+			browseFailure("listing a database's branches for the overview failed", err)
 		}
 	} else {
-		browseErr = err.Error()
+		browseFailure("opening a database for the overview failed", err)
 	}
 
 	view := struct {


@@ 232,7 248,7 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) {
 		Commits       []browse.CommitInfo
 		Views         []View
 		CloneURL      string
-		BrowseError   string
+		BrowseFailed  bool
 	}{
 		Page:          a.page(r, repo.OwnerName+"/"+repo.Name+" — "+serviceName),
 		Repo:          repo,


@@ 241,7 257,7 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) {
 		Commits:       commits,
 		Views:         views,
 		CloneURL:      a.cloneURL(repo),
-		BrowseError:   browseErr,
+		BrowseFailed:  browseFailed,
 	}
 	a.render(w, http.StatusOK, "overview", view)
 }

@@ 41,8 41,8 @@ dolt clone {{.CloneURL}}</pre>
   </div>
   <div class="col-md-8">
     <h4>{{icon "clock"}} Recent commits</h4>
-    {{if .BrowseError}}
-    <div class="alert alert-warning">Could not read history: {{.BrowseError}}</div>
+    {{if .BrowseFailed}}
+    <div class="alert alert-warning">Could not read history.</div>
     {{end}}
     {{if .Commits}}
     <table class="table">

@@ 544,6 544,69 @@ func TestAMissingDatabaseIsStillNotFound(t *testing.T) {
 		"a database somebody may not see and one that is not there must render the same page")
 }
 
+// browseDetail is a browse failure of the shape the layer really produces: a
+// dolt internal, and the store's path on our disk.
+const browseDetail = "browse: walk commits: open /var/lib/dolt/~alice/db/.dolt/noms/oldgen: no such file"
+
+// The overview used to render the browse layer's own error text into the page,
+// under "Could not read history: ". A reader can do nothing with a chunk store's
+// path, and nothing else on this surface discloses one. The page carries a fixed
+// sentence and the detail goes to the log.
+func TestOverviewDoesNotPrintTheBrowseError(t *testing.T) {
+	newOverview := func(t *testing.T) *harness {
+		t.Helper()
+		h := newHarness(t)
+		h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice",
+			Path: "/var/lib/dolt/~alice/db", Visibility: core.VisibilityPublic})
+		return h
+	}
+	assertHidden := func(t *testing.T, body string) {
+		t.Helper()
+		assert.Contains(t, body, "Could not read history.")
+		assert.NotContains(t, body, browseDetail)
+		assert.NotContains(t, body, "/var/lib/dolt", "the store's path must not reach the reader")
+		assert.NotContains(t, body, "walk commits", "dolt's internals must not reach the reader")
+	}
+
+	t.Run("a log that cannot be read", func(t *testing.T) {
+		h := newOverview(t)
+		h.browse.sess = &fakeSession{
+			branches: []browse.Branch{{Name: "main", Head: "abcdef1234567890"}},
+			logErr:   errors.New(browseDetail),
+		}
+
+		rec := h.do("GET", "/~alice/db", nil, nil)
+		require.Equal(t, http.StatusOK, rec.Code)
+		body := rec.Body.String()
+		assertHidden(t, body)
+		// The rest of the page is still the page: a history we cannot read is
+		// not a reason to withhold the branches we can.
+		assert.Contains(t, body, "main")
+	})
+
+	t.Run("a store that cannot be opened", func(t *testing.T) {
+		h := newOverview(t)
+		h.browse.errByPath = map[string]error{"/var/lib/dolt/~alice/db": errors.New(browseDetail)}
+
+		rec := h.do("GET", "/~alice/db", nil, nil)
+		require.Equal(t, http.StatusOK, rec.Code)
+		assertHidden(t, rec.Body.String())
+	})
+
+	// An empty database is a state and not a failure, and the page said so
+	// before this change too. It must keep saying it, with no warning attached.
+	t.Run("an empty database is not a failure", func(t *testing.T) {
+		h := newOverview(t)
+		h.browse.sess = &fakeSession{branches: []browse.Branch{{Name: "main", Head: "abcdef1234567890"}}}
+
+		rec := h.do("GET", "/~alice/db", nil, nil)
+		require.Equal(t, http.StatusOK, rec.Code)
+		body := rec.Body.String()
+		assert.Contains(t, body, "No commits.")
+		assert.NotContains(t, body, "Could not read history")
+	})
+}
+
 func TestDashboardLists(t *testing.T) {
 	h := newHarness(t)
 	h.store.add(&core.Repo{Name: "mine", OwnerID: 3, OwnerName: "bob", Path: "/m", Visibility: core.VisibilityPrivate})