amadan

carlos / platform Public

tests: refusals, positive controls and edge cases the reviews asked for

- internal/console/apierrors_test.go: TestAPIErrorsRefusals — a
  non-member's token gets the byte-identical 404 on GET
  /api/cli/apps/{sqid}/titogo/errors; since/source/fp/level each 400
  on a bad value, mirroring TestAPILogsRefusals.
- internal/edge/edgeerrors_test.go:
  - TestEdgeEmitsUnmarked503OnHibernatingRoute — positive control for
    TestEdgeSkipsActivatorAnswered503: an Activator that writes the
    same wake-path 503 but never calls MarkAnswered must be filed as
    exactly one edge error.
  - memErrSink gained a snap() method that copies under its mutex;
    every assertion in the file now reads through it.
  - TestEdgeEmitsErrorOnUpstream5xx now asserts attribution:
    sink.apps[0] == release.OpsAccount+"/web", since the route names
    no Account and RouteAccount falls back to OpsAccount.
- internal/apperr/read_test.go: TestReadWindowListsHourBeforeSince —
  a record opened in hour 10 and still open past 11:00 is found when
  Since is 11:00 (the listing widens one hour behind Since); a
  record in the same chunk whose Last never reaches the window is
  excluded.
- internal/apperr/sink_test.go: TestSinkDrainOverflowOnly — fill an
  app to the cap, Drain, then Add one new key: Drain returns exactly
  one record, FP == OverflowFP, Count == 1 (the per-hour cap counter
  survives a Drain).
- internal/applog/chunk_test.go: TestHourNameOfDir — trailing slash,
  no trailing slash, a bare name, and the empty string.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 2c1c62a2da2999d4686d19fc3eaff9bf9accdcd4 parent 355aaa3
5 files changed, +156 −5
  • internal/apperr/read_test.go +36 −0
  • internal/apperr/sink_test.go +24 −0
  • internal/applog/chunk_test.go +13 −0
  • internal/console/apierrors_test.go +19 −0
  • internal/edge/edgeerrors_test.go +64 −5

CI passed — run details

diff --git a/internal/apperr/read_test.go b/internal/apperr/read_test.go
index 7a911432b..deed9a676 100644
--- a/internal/apperr/read_test.go
+++ b/internal/apperr/read_test.go
@@ -49,3 +49,39 @@ func TestReadWindowMergesAndFilters(t *testing.T) {
t.Fatalf("host filter = %+v", recs)
}
}
+
+// TestReadWindowListsHourBeforeSince: a record's chunk sits under the hour
+// of its FIRST occurrence, which can be well before the window if the group
+// is still open. Since 11:00 must still find a chunk filed under hour 10 —
+// the listing is widened by one hour behind Since — while a record in that
+// same chunk whose Last never reaches the window stays excluded by Match.
+func TestReadWindowListsHourBeforeSince(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ base := time.Date(2026, 9, 7, 10, 50, 0, 0, time.UTC) // hour dir "…-10"
+ recs := []Record{
+ // Opened at 10:50, still being added to at 11:05: inside the
+ // [Since,Until] window by overlap, but filed under hour 10.
+ {TS: base, Box: "box1", Host: "h1", Source: SourceEdge, Name: "long", FP: "flong",
+ Count: 5, First: base, Last: base.Add(15 * time.Minute), Seq: 1},
+ // Opened and closed at 10:50-10:55, entirely before the window.
+ {TS: base, Box: "box1", Host: "h1", Source: SourceEdge, Name: "gone", FP: "fgone",
+ Count: 1, First: base, Last: base.Add(5 * time.Minute), Seq: 2},
+ }
+ body, err := EncodeChunk(recs)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := s.Put(ctx, ChunkKey("bab", "web", "box1", HourOf(recs[0].TS), recs[0].Seq, len(body)), body, ""); err != nil {
+ t.Fatal(err)
+ }
+ since := time.Date(2026, 9, 7, 11, 0, 0, 0, time.UTC)
+ until := time.Date(2026, 9, 7, 11, 30, 0, 0, time.UTC)
+ got, trimmed, err := ReadWindow(ctx, s, "bab", "web", Filter{Since: since, Until: until}, 100)
+ if err != nil || trimmed {
+ t.Fatalf("read: %v %v", err, trimmed)
+ }
+ if len(got) != 1 || got[0].FP != "flong" {
+ t.Fatalf("window = %+v, want only flong", got)
+ }
+}
diff --git a/internal/apperr/sink_test.go b/internal/apperr/sink_test.go
index 60dc22930..8fa9a3bcf 100644
--- a/internal/apperr/sink_test.go
+++ b/internal/apperr/sink_test.go
@@ -71,6 +71,30 @@ func TestSinkCapsGroupsPerAppPerHour(t *testing.T) {
}
}
+// TestSinkDrainOverflowOnly: Drain resets the groups but not the per-hour
+// `seen` cap counter, so an app that was already at the cap when last
+// drained stays at the cap — the next genuinely new key goes straight to
+// overflow, and a drain right after that Add must show nothing else.
+func TestSinkDrainOverflowOnly(t *testing.T) {
+ now := time.Date(2026, 9, 7, 10, 0, 0, 0, time.UTC)
+ s := NewSink("box1", func() time.Time { return now })
+ for i := 0; i < MaxGroupsPerAppHour; i++ {
+ s.Add("bab", "web", Record{Source: SourceEdge, Kind: KindHTTP, Name: fmt.Sprintf("500 GET /p%d-x", i), FP: fmt.Sprintf("fp%d", i)})
+ }
+ if recs := s.Drain()[applog.AppKey{Acct: "bab", App: "web"}]; len(recs) != MaxGroupsPerAppHour {
+ t.Fatalf("fill drain = %d, want %d", len(recs), MaxGroupsPerAppHour)
+ }
+ // One brand-new key, nothing else: the cap counter survived the drain.
+ s.Add("bab", "web", Record{Source: SourceEdge, Kind: KindHTTP, Name: "brand new", FP: "brandnew"})
+ recs := s.Drain()[applog.AppKey{Acct: "bab", App: "web"}]
+ if len(recs) != 1 {
+ t.Fatalf("overflow-only drain = %d records, want 1: %+v", len(recs), recs)
+ }
+ if recs[0].FP != OverflowFP || recs[0].Count != 1 {
+ t.Fatalf("record = %+v", recs[0])
+ }
+}
+
func TestSinkSanitisesAndFingerprintsOnAdd(t *testing.T) {
now := time.Date(2026, 9, 7, 10, 0, 0, 0, time.UTC)
s := NewSink("box1", func() time.Time { return now })
diff --git a/internal/applog/chunk_test.go b/internal/applog/chunk_test.go
index b257ebe4b..9ab61b891 100644
--- a/internal/applog/chunk_test.go
+++ b/internal/applog/chunk_test.go
@@ -105,6 +105,19 @@ func TestDecodeChunkSkipsMalformedLines(t *testing.T) {
}
}
+func TestHourNameOfDir(t *testing.T) {
+ for dir, want := range map[string]string{
+ "apps/x/y/logs/box/2026-09-07-10/": "2026-09-07-10",
+ "apps/x/y/logs/box/2026-09-07-10": "2026-09-07-10",
+ "plain": "plain",
+ "": "",
+ } {
+ if got := HourNameOfDir(dir); got != want {
+ t.Errorf("HourNameOfDir(%q) = %q, want %q", dir, got, want)
+ }
+ }
+}
+
func TestHourBefore(t *testing.T) {
cutoff := time.Date(2026, 8, 10, 21, 30, 0, 0, time.UTC)
for hour, want := range map[string]bool{
diff --git a/internal/console/apierrors_test.go b/internal/console/apierrors_test.go
index 41ef56a3e..04e181c39 100644
--- a/internal/console/apierrors_test.go
+++ b/internal/console/apierrors_test.go
@@ -86,6 +86,25 @@ func TestAPIErrorsGroupsBothFamilies(t *testing.T) {
}
}
+// TestAPIErrorsRefusals mirrors TestAPILogsRefusals (apilogs_test.go): a
+// non-member's read is the family's byte-identical 404; bad parameters
+// name themselves in a 400.
+func TestAPIErrorsRefusals(t *testing.T) {
+ fx, _ := instanceFixture(t, "code-errs-refusal")
+ eve := cliToken(t, fx, "eve@keymail.dev", "code-errs-eve")
+ want := canonicalNotFound(t)
+ status, h, body := rawReq(t, http.MethodGet, errorsURL(fx, fx.sqid, "titogo", ""), eve, "")
+ assertNotFound(t, "errors as non-member", want, status, h, body)
+
+ token := cliToken(t, fx, "alice@keymail.dev", "code-errs-token")
+ for _, q := range []string{"since=nonsense", "source=mars", "fp=has space", "level=loud"} {
+ status, _, body := rawReq(t, http.MethodGet, errorsURL(fx, fx.sqid, "titogo", q), token, "")
+ if status != http.StatusBadRequest {
+ t.Errorf("?%s = %d, want 400 (%s)", q, status, body)
+ }
+ }
+}
+
func TestAPITelemetryPutSetsLogLevel(t *testing.T) {
fx, token := instanceFixture(t, "code-errors-2")
now := time.Now().UTC()
diff --git a/internal/edge/edgeerrors_test.go b/internal/edge/edgeerrors_test.go
index f6c20bf5f..e267de7d1 100644
--- a/internal/edge/edgeerrors_test.go
+++ b/internal/edge/edgeerrors_test.go
@@ -9,6 +9,7 @@ import (
"github.com/carlosframework/platform/internal/apperr"
"github.com/carlosframework/platform/internal/registry"
+ "github.com/carlosframework/platform/internal/release"
)
type memErrSink struct {
@@ -24,6 +25,17 @@ func (m *memErrSink) Add(acct, app string, r apperr.Record) {
m.apps = append(m.apps, acct+"/"+app)
}
+// snap copies the sink's records under the lock — every assertion reads
+// through this rather than the fields directly, so a test can never
+// observe a torn append from a concurrent Add.
+func (m *memErrSink) snap() []apperr.Record {
+ m.mu.Lock()
+ defer m.mu.Unlock()
+ out := make([]apperr.Record, len(m.recs))
+ copy(out, m.recs)
+ return out
+}
+
func TestEdgeEmitsErrorOnUpstream5xx(t *testing.T) {
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
@@ -64,16 +76,22 @@ func TestEdgeEmitsErrorOnUpstream5xx(t *testing.T) {
if do("POST", "/boom?token=abc") != 500 || do("GET", "/missing") != 404 || do("GET", "/") != 200 {
t.Fatal("upstream statuses must pass through unchanged")
}
- if len(sink.recs) != 1 {
- t.Fatalf("errors emitted = %d, want 1 (the 500 only)", len(sink.recs))
+ recs := sink.snap()
+ if len(recs) != 1 {
+ t.Fatalf("errors emitted = %d, want 1 (the 500 only)", len(recs))
}
- r := sink.recs[0]
+ r := recs[0]
if r.Source != apperr.SourceEdge || r.Kind != apperr.KindHTTP || r.Status != 500 || r.Host != "app.example.com" || r.Release != "v7" {
t.Fatalf("record = %+v", r)
}
if r.Name != "500 POST /boom" || r.Path != "/boom" || strings.Contains(r.Name, "token") {
t.Fatalf("name/path = %q %q", r.Name, r.Path)
}
+ // The route carries no Account, so RouteAccount falls back to
+ // OpsAccount: attribution must land on the ops account, not "".
+ if sink.apps[0] != release.OpsAccount+"/web" {
+ t.Fatalf("attribution = %q, want %q", sink.apps[0], release.OpsAccount+"/web")
+ }
}
// answering503 is an Activator that answers every request itself with the
@@ -107,7 +125,48 @@ func TestEdgeSkipsActivatorAnswered503(t *testing.T) {
if rec.Code != 503 {
t.Fatalf("status = %d", rec.Code)
}
- if len(sink.recs) != 0 {
- t.Fatalf("an activator-answered 503 is already a platform log error; got %d edge errors", len(sink.recs))
+ if recs := sink.snap(); len(recs) != 0 {
+ t.Fatalf("an activator-answered 503 is already a platform log error; got %d edge errors", len(recs))
+ }
+}
+
+// plain503 is answering503's negative twin: it writes the same wake-path
+// 503 but never calls MarkAnswered, so the edge must not mistake it for the
+// activator's own already-logged error.
+type plain503 struct{}
+
+func (plain503) ServeHTTP(w http.ResponseWriter, r *http.Request, rt registry.Route, proxy http.Handler) bool {
+ http.Error(w, "instance failed to wake", http.StatusServiceUnavailable)
+ return true
+}
+func (plain503) RetireExcept(map[string]bool) {}
+
+// TestEdgeEmitsUnmarked503OnHibernatingRoute is the positive control for
+// TestEdgeSkipsActivatorAnswered503: with the MarkAnswered call gone, the
+// same hibernating route's 503 must be filed as exactly one edge error.
+func TestEdgeEmitsUnmarked503OnHibernatingRoute(t *testing.T) {
+ reg := openReg(t)
+ rt := registry.Route{Host: "sleepy.example.com", Kind: "instance", Socket: t.TempDir() + "/x.sock", Status: "ready", Hibernate: true, App: "web"}
+ if err := reg.Upsert(rt); err != nil {
+ t.Fatal(err)
+ }
+ e, err := New(reg, plain503{})
+ if err != nil {
+ t.Fatal(err)
+ }
+ sink := &memErrSink{}
+ e.EnableErrors(sink)
+ rec := httptest.NewRecorder()
+ req := httptest.NewRequest("GET", "http://sleepy.example.com/", nil)
+ e.ServeHTTP(rec, req)
+ if rec.Code != 503 {
+ t.Fatalf("status = %d", rec.Code)
+ }
+ recs := sink.snap()
+ if len(recs) != 1 {
+ t.Fatalf("errors emitted = %d, want 1 (the unmarked 503)", len(recs))
+ }
+ if recs[0].Status != 503 || recs[0].Source != apperr.SourceEdge {
+ t.Fatalf("record = %+v", recs[0])
}
}