M web/handlers_settings.go => web/handlers_settings.go +5 -1
@@ 21,6 21,10 @@ import (
// repo, or a hidden PRIVATE repo the caller cannot even browse, is reported as
// not found; a visible repo the caller does not own is a plain 403. On any of
// these it writes the response and returns ok=false.
+//
+// The lookup is classified by repoLookupFailed, exactly as the browse path is: a
+// settings URL that answered "no such database" while the metadata store was
+// unreachable would be the same lie told on a different page.
func (a *app) loadRepoForAdmin(w http.ResponseWriter, r *http.Request) (repo *core.Repo, ac *authContext, ok bool) {
ac = a.requireLogin(w, r)
if ac == nil {
@@ 32,7 36,7 @@ func (a *app) loadRepoForAdmin(w http.ResponseWriter, r *http.Request) (repo *co
name := chi.URLParam(r, "db")
repo, err := a.cfg.Repos.GetRepoByOwnerAndName(r.Context(), owner, name)
if err != nil {
- a.notFound(w, r)
+ a.repoLookupFailed(w, r, err)
return nil, nil, false
}
M web/router.go => web/router.go +47 -2
@@ 1,14 1,18 @@
package web
import (
+ "errors"
"fmt"
"html/template"
"io/fs"
+ "log/slog"
"net/http"
"os"
"github.com/go-chi/chi/v5"
+ "go.bigb.es/auxilia/scribe"
+
"sourcecraft.dev/bigbes/sr-ht-ecore/assets"
"sourcecraft.dev/bigbes/sr-ht-ecore/chimw"
"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
@@ 20,6 24,7 @@ import (
"sourcecraft.dev/bigbes/sr-ht-dolt/authn"
"sourcecraft.dev/bigbes/sr-ht-dolt/beads"
"sourcecraft.dev/bigbes/sr-ht-dolt/core"
+ "sourcecraft.dev/bigbes/sr-ht-dolt/db"
)
// serviceName is our own service key: the config section, the JWT audience and
@@ 308,6 313,18 @@ func (a *app) forbidden(w http.ResponseWriter, r *http.Request, msg string) {
a.fail(w, r, http.StatusForbidden, msg)
}
+// internalError renders the shared 500 page and logs the cause. The split is the
+// whole point: the reason a request could not be answered names hosts, queries
+// and on-disk paths, so it goes to the operator's log, and the reader gets the
+// same fixed sentence every other bug on this surface produces.
+//
+// scribe.Err rather than %v, as the newer handlers here do: it expands a culpa
+// chain into err.msg, err.code and err.hint instead of flattening it.
+func (a *app) internalError(w http.ResponseWriter, r *http.Request, what string, err error) {
+ slog.Error(what, "component", "web", "path", r.URL.Path, scribe.Err(err))
+ a.fail(w, r, http.StatusInternalServerError, "")
+}
+
// redirectLogin sends an unauthenticated caller to meta's login, returning them
// to the current URL afterwards.
//
@@ 323,6 340,9 @@ func (a *app) redirectLogin(w http.ResponseWriter, r *http.Request) {
// (404 for hidden PRIVATE repos, 403 otherwise) and returns ok=false. On
// success it returns the repo, the (possibly nil) caller and the caller's ACL
// grant for reuse by the handler.
+//
+// "Not there" and "could not be looked up" are two answers and not one; see
+// repoLookupFailed.
func (a *app) loadRepoForBrowse(w http.ResponseWriter, r *http.Request) (repo *core.Repo, caller *core.Caller, aclMode *core.AccessMode, ok bool) {
owner := chi.URLParam(r, "user")
name := chi.URLParam(r, "db")
@@ 331,8 351,7 @@ func (a *app) loadRepoForBrowse(w http.ResponseWriter, r *http.Request) (repo *c
repo, err := a.cfg.Repos.GetRepoByOwnerAndName(r.Context(), owner, name)
if err != nil {
- // A missing repo is reported as not found regardless of the caller.
- a.notFound(w, r)
+ a.repoLookupFailed(w, r, err)
return nil, nil, nil, false
}
@@ 348,6 367,32 @@ func (a *app) loadRepoForBrowse(w http.ResponseWriter, r *http.Request) (repo *c
return repo, caller, aclMode, true
}
+// repoLookupFailed answers a failed {user}/{db} lookup, and is the one place
+// this surface decides which of the two failures it was.
+//
+// - db.ErrNotFound is an answer about the data: there is no such row. It is a
+// 404, and it has to stay the *same* 404 the visibility rule renders for a
+// PRIVATE database the caller may not see (core.NotFoundForPrivate, in the
+// callers below) — a database somebody may not look at must be
+// indistinguishable from one that is not there, which is the whole reason
+// the status was chosen.
+// - Anything else is not an answer at all: the metadata store could not be
+// read. Reporting that as a 404 tells a reader their database does not
+// exist when the truth is that we cannot say right now, and it does it on
+// every page of every database on the instance for as long as Postgres is
+// down. It is a 500, with the cause logged and never rendered.
+//
+// The default arm is the 500 on purpose: an unmapped error is a bug in a layer
+// below, and rendering it as "not found" would report that bug to the reader as
+// a fact about their data.
+func (a *app) repoLookupFailed(w http.ResponseWriter, r *http.Request, err error) {
+ if errors.Is(err, db.ErrNotFound) {
+ a.notFound(w, r)
+ return
+ }
+ a.internalError(w, r, "looking up a database failed", err)
+}
+
// effectiveACL resolves the caller's ACL grant on repo, or nil for an anonymous
// caller or a caller with no grant. A lookup error degrades to nil (no grant):
// access then falls back to visibility, which never over-grants.
M web/web_test.go => web/web_test.go +72 -1
@@ 49,7 49,12 @@ type fakeStore struct {
createErr error
// listErr, when set, makes ListReposForViewer fail — the metadata store
// unreachable, which the pages that enumerate databases have to survive.
- listErr error
+ listErr error
+ // getErr, when set, makes GetRepoByOwnerAndName fail with it instead of
+ // answering — the metadata store unreachable rather than the row missing,
+ // which the pages that resolve one database must not confuse with a
+ // database that does not exist.
+ getErr error
createdCalls []*core.Repo
deletedRepos []int
}
@@ 87,6 92,9 @@ func (f *fakeStore) CreateRepo(_ context.Context, r *core.Repo) (*core.Repo, err
}
func (f *fakeStore) GetRepoByOwnerAndName(_ context.Context, owner, name string) (*core.Repo, error) {
+ if f.getErr != nil {
+ return nil, f.getErr
+ }
r, ok := f.repos[owner+"/"+name]
if !ok {
return nil, db.ErrNotFound
@@ 473,6 481,69 @@ func TestPrivateVisibleToOwner(t *testing.T) {
}
}
+// storeOutage is the shape of a metadata store that cannot answer: not a miss,
+// and carrying a host and a port the reader has no business seeing.
+var storeOutage = errors.New("dial tcp 10.0.0.5:5432: connect: connection refused")
+
+// A database whose metadata row cannot be *read* is not a database that does not
+// exist. Reporting the outage as a 404 tells every reader on the instance that
+// their database is gone for as long as Postgres is down — and, being a 404 with
+// the shared sentence, tells them so in the voice reserved for "there is nothing
+// here".
+func TestAStoreOutageIsNotAMissingDatabase(t *testing.T) {
+ for _, tc := range []struct {
+ name, target string
+ caller *auth.AuthContext
+ }{
+ {"overview", "/~alice/db", nil},
+ {"log", "/~alice/db/log", nil},
+ {"tree", "/~alice/db/tree/main", nil},
+ {"table", "/~alice/db/table/main/things", nil},
+ {"view", "/~alice/db/view/beads", nil},
+ // The admin path resolves the same row through loadRepoForAdmin and must
+ // classify it the same way.
+ {"settings", "/~alice/db/settings", testCaller(1, "alice")},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ h := newHarness(t)
+ h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic})
+ h.store.getErr = storeOutage
+
+ rec := h.do("GET", tc.target, tc.caller, nil)
+ require.Equal(t, http.StatusInternalServerError, rec.Code)
+
+ body := rec.Body.String()
+ assert.Contains(t, body, pages.InternalMessage)
+ assert.NotContains(t, body, "connection refused", "the cause belongs in the log")
+ assert.NotContains(t, body, "10.0.0.5:5432")
+ assert.NotContains(t, body, pages.NotFoundMessage)
+ })
+ }
+}
+
+// The other arm, and the one the masking rule rests on: the store's own "no such
+// row" — including a wrapped one, which is how db/ hands it up — is still the
+// 404, and still the *same* 404 a PRIVATE database the caller may not see gets.
+func TestAMissingDatabaseIsStillNotFound(t *testing.T) {
+ h := newHarness(t)
+ h.store.add(&core.Repo{Name: "sec", OwnerID: 1, OwnerName: "alice", Path: "/s", Visibility: core.VisibilityPrivate})
+
+ // The masked PRIVATE database, and the same URL with no row behind it at
+ // all. One URL for both so the two responses are comparable byte for byte:
+ // the shared error page carries the request's own path in the login link.
+ hidden := h.do("GET", "/~alice/sec", nil, nil)
+ require.Equal(t, http.StatusNotFound, hidden.Code)
+ assert.Contains(t, hidden.Body.String(), pages.NotFoundMessage)
+
+ // A miss the store wrapped on its way up, which errors.Is must still see
+ // through — db/ wraps its misses with the owner and name it looked up.
+ h.store.getErr = fmt.Errorf("db: get repo ~alice/sec: %w", db.ErrNotFound)
+ missing := h.do("GET", "/~alice/sec", nil, nil)
+ require.Equal(t, http.StatusNotFound, missing.Code)
+ assert.Equal(t, hidden.Body.String(), missing.Body.String(),
+ "a database somebody may not see and one that is not there must render the same page")
+}
+
func TestDashboardLists(t *testing.T) {
h := newHarness(t)
h.store.add(&core.Repo{Name: "mine", OwnerID: 3, OwnerName: "bob", Path: "/m", Visibility: core.VisibilityPrivate})