console: fix round 1 for task 9 — seed Sources before the early returns
readAnalytics set out.Sources = []string{"edge"} only after the two
early returns (no release store; ReadRange failed), so a bucket-less
console rendered "Sources live: ." and the API answered
"sources":[] — contradicting the contract that "edge" is always live.
Move the seed to the top of the function, right after out is built;
"browser"'s append and DroppedTotal stay where they were, since both
need Sum.
Also, two small fixes from review:
- apianalytics_test.go's TestAppAnalyticsPageDroppedByReason checked
for the bare substring "3", which the page's own range selector
("30 days") already satisfies regardless of any drop — rebuilt the
test to seed exact deltas directly (3 origin drops + 1 flush, total
4) and assert the precise rendered fragments: "origin 3, malformed
0" and the headline Dropped cell "<td>4</td>".
- Added an HTML comment above each entity-escaped snippet line in
app_analytics.html and app_errors.html explaining why src="..." is
spelled with =/" there, for the next reader who wonders.
New test: TestReadAnalyticsSourcesNoBucket exercises readAnalytics
directly with cfg.Releases == nil and asserts Sources == ["edge"].
It does not go through the Analytics tab's HTTP handler: appPage's
channel assembly (Assemble → assembleChannels) calls
s.cfg.Releases.List unconditionally and panics on a nil store for
ANY tab, not just Analytics — a separate, pre-existing gap outside
this task's scope. Noted in the test's own comment.
go build ./... && gofmt -l internal/ . && go vet ./internal/console/
&& go test ./internal/console/ ./internal/docsite/ . all clean/pass.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4 files changed,
+61
−21
internal/console/analyticsview.go+12 −7internal/console/apianalytics_test.go+41 −14internal/console/templates/app_analytics.html+4 −0internal/console/templates/app_errors.html+4 −0
CI passed — run details
diff --git a/internal/console/analyticsview.go b/internal/console/analyticsview.go| index 122abcc6a..19c35da86 100644 |
| --- a/internal/console/analyticsview.go |
| +++ b/internal/console/analyticsview.go |
| @@ -252,6 +252,13 @@ const ( |
| // it was on and hiding them would read as data loss. |
| func (s *Server) readAnalytics(ctx context.Context, sqid, app, rng string, now time.Time) (analyticsWindow, error) { |
| out := analyticsWindow{Range: rng} |
| + // "edge" is always live — page views are counted there whether or not |
| + // counting is on, and this must hold on every path out of this read, |
| + // including a bucket-less console or a failed range read below: a |
| + // caller must never see "Sources live: ." or an empty "sources" array. |
| + // "browser" joins later, once the summary shows the beacon has |
| + // actually been reached. |
| + out.Sources = []string{"edge"} |
| res, err := telemetry.Resolve(ctx, s.cfg.Releases, sqid, app) |
| if err != nil { |
| return out, err |
| @@ -269,13 +276,11 @@ func (s *Server) readAnalytics(ctx context.Context, sqid, app, rng string, now t |
| return out, err |
| } |
| out.Sum = sum |
| - // "edge" is always live — page views are counted there whether or not |
| - // counting is on. "browser" joins only once the summary shows the |
| - // beacon has actually been reached: a named event, or a drop at either |
| - // door (a shut door still proves the snippet made it to the page and |
| - // tried). An app nobody has pointed the snippet at yet must not claim a |
| - // source it has not seen (ruling 7). |
| - out.Sources = []string{"edge"} |
| + // "browser" joins only once the summary shows the beacon has actually |
| + // been reached: a named event, or a drop at either door (a shut door |
| + // still proves the snippet made it to the page and tried). An app |
| + // nobody has pointed the snippet at yet must not claim a source it has |
| + // not seen (ruling 7). |
| d := out.Sum.Dropped |
| out.DroppedTotal = d.E.Rate + d.E.Size + d.E.Origin + d.E.Malformed + d.A.Rate + d.A.Size + d.A.Origin + d.A.Malformed + d.Flush |
| if len(out.Sum.Events) > 0 || out.DroppedTotal-d.Flush > 0 { |
diff --git a/internal/console/apianalytics_test.go b/internal/console/apianalytics_test.go| index f625178a0..af86370a1 100644 |
| --- a/internal/console/apianalytics_test.go |
| +++ b/internal/console/apianalytics_test.go |
| @@ -18,6 +18,7 @@ import ( |
| "github.com/carlosframework/platform/internal/release" |
| "github.com/carlosframework/platform/internal/svcrole" |
| "github.com/carlosframework/platform/internal/telemetry" |
| + "github.com/carlosframework/platform/internal/usage" |
| ) |
| func TestAPITelemetryPutSetsAnalytics(t *testing.T) { |
| @@ -192,6 +193,30 @@ func TestAppAnalyticsPage(t *testing.T) { |
| } |
| } |
| +// TestReadAnalyticsSourcesNoBucket: "edge" is always live, even on a |
| +// console with nowhere to read a summary from at all. readAnalytics |
| +// returns before ever touching Sum on this path (s.cfg.Releases == nil), |
| +// so the seed has to happen before that early return — a caller must |
| +// never see an empty Sources slice here. |
| +// |
| +// This exercises readAnalytics directly rather than the rendered page: |
| +// going through the Analytics tab's own HTTP handler would also exercise |
| +// appPage's channel assembly (Assemble → assembleChannels), which calls |
| +// s.cfg.Releases.List unconditionally and is not nil-store-safe for ANY |
| +// tab — a separate, pre-existing gap outside this task's scope, not |
| +// something the door/sources feature introduced or can fix in passing. |
| +func TestReadAnalyticsSourcesNoBucket(t *testing.T) { |
| + fx, _ := instanceFixture(t, "code-analytics-nobucket") |
| + fx.srv.cfg.Releases = nil |
| + win, err := fx.srv.readAnalytics(context.Background(), fx.sqid, "titogo", rangeToday, seedTime()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if want := []string{"edge"}; len(win.Sources) != len(want) || win.Sources[0] != want[0] { |
| + t.Fatalf("no-bucket sources = %v, want %v", win.Sources, want) |
| + } |
| +} |
| + |
| // TestReadAnalyticsSources: "edge" is always live; "browser" joins only |
| // once the summary shows the beacon was actually reached — a named event, |
| // in this case (ruling 7). An app nobody has pointed the snippet at yet |
| @@ -221,21 +246,23 @@ func TestReadAnalyticsSources(t *testing.T) { |
| } |
| // TestAppAnalyticsPageDroppedByReason: a window with browser door drops |
| -// shows the breakdown, names the total in the headline, and still claims |
| -// "browser" as a live source even with no named events — a shut door that |
| -// is dropping beacons has, by definition, been reached. |
| +// shows the breakdown, names the honest total in the headline (not just |
| +// Flush), and still claims "browser" as a live source even with no named |
| +// events — a shut door that is dropping beacons has, by definition, been |
| +// reached. Two deltas, built directly rather than through Meter.Drop, so |
| +// the exact counters (and their sum) are pinned rather than incidental: |
| +// three errors-door origin drops and one flush drop, total 4. |
| func TestAppAnalyticsPageDroppedByReason(t *testing.T) { |
| fx, _ := instanceFixture(t, "code-analytics-dropped") |
| when := seedTime() |
| - m := analytics.NewMeter(t.TempDir(), "box1", func() time.Time { return when }) |
| - m.Drop(release.OpsAccount, "titogo", analytics.DoorErrors, analytics.DropOrigin, when) |
| - m.Drop(release.OpsAccount, "titogo", analytics.DoorErrors, analytics.DropOrigin, when) |
| - m.Drop(release.OpsAccount, "titogo", analytics.DoorErrors, analytics.DropOrigin, when) |
| - for k, ds := range m.Drain() { |
| - for _, d := range ds { |
| - if err := analytics.UpsertDay(context.Background(), fx.rel, k.Acct, k.App, d); err != nil { |
| - t.Fatal(err) |
| - } |
| + date, box := usage.DayOf(when), "box1" |
| + deltas := []analytics.Delta{ |
| + {Date: date, Box: box, Seq: 1, Dropped: analytics.Dropped{E: analytics.DropCounts{Origin: 3}}}, |
| + {Date: date, Box: box, Seq: 2, Dropped: analytics.Dropped{Flush: 1}}, |
| + } |
| + for _, d := range deltas { |
| + if err := analytics.UpsertDay(context.Background(), fx.rel, release.OpsAccount, "titogo", d); err != nil { |
| + t.Fatal(err) |
| } |
| } |
| if err := telemetry.Update(context.Background(), fx.rel, telemetry.AppKey(fx.sqid, "titogo"), func(st *telemetry.Settings) { |
| @@ -250,9 +277,9 @@ func TestAppAnalyticsPageDroppedByReason(t *testing.T) { |
| if status != 200 { |
| t.Fatalf("%d %s", status, body) |
| } |
| - for _, want := range []string{"origin", "3", "Dropped by reason", "Sources live: edge, browser"} { |
| + for _, want := range []string{"origin 3, malformed 0", "Dropped by reason", "Sources live: edge, browser", "<td>4</td>"} { |
| if !strings.Contains(body, want) { |
| - t.Errorf("page missing %q", want) |
| + t.Errorf("page missing %q:\n%s", want, body) |
| } |
| } |
| } |
diff --git a/internal/console/templates/app_analytics.html b/internal/console/templates/app_analytics.html| index ff51f213b..2e7ceb037 100644 |
| --- a/internal/console/templates/app_analytics.html |
| +++ b/internal/console/templates/app_analytics.html |
| @@ -41,6 +41,10 @@ account on the <a href="{{.Base}}/apps/{{.AppName}}/settings">Settings</a> tab.< |
| {{template "kvtable" .Sum.Referers}} |
| <h3>Named events{{if .Sum.EventsApprox}} <span class="small">approximate</span>{{end}}</h3> |
| {{template "kvtable" .Sum.Events}} |
| +<!-- The script tag below is prose, not a real subresource fetch: its src |
| + attribute is spelled with = for = and " for the quotes so |
| + TestConsoleTemplatesFetchNothingOffOrigin (icons_test.go) does not |
| + misread this line as one. --> |
| <p class="small">Named events and JavaScript errors need one line in the page: <code><script src="/__carlos/carlos.js" defer></script></code>. Then <code>carlos.track("signup", {plan: "pro"})</code> counts an event here (fields are accepted and not yet shown). Page views are counted at the edge already; the snippet never sends them. What edge counting misses: single-page-app route changes, pages served by a CDN in front of CARLOS, and back/forward-cache restores.</p> |
| {{end}} |
| <p class="small">Times are UTC. The same view from a terminal: <code>carlos analytics --app {{.AppName}} --range {{.AnalyticsRange}}</code>.</p> |
diff --git a/internal/console/templates/app_errors.html b/internal/console/templates/app_errors.html| index c9d77fc95..e929ce41b 100644 |
| --- a/internal/console/templates/app_errors.html |
| +++ b/internal/console/templates/app_errors.html |
| @@ -74,6 +74,10 @@ Counts are to the nearest flush. Messages are redacted for addresses and credent |
| <p class="small">Times are UTC. The same view from a terminal: <code>carlos errors --app {{.AppName}} --since {{.ErrSince}}</code>.</p> |
| {{else if not .ErrsErr}} |
| <p>No errors in this window.</p> |
| +<!-- The script tag below is prose, not a real subresource fetch: its src |
| + attribute is spelled with = for = and " for the quotes so |
| + TestConsoleTemplatesFetchNothingOffOrigin (icons_test.go) does not |
| + misread this line as one. --> |
| <p class="small">Log lines at <code>{{.ErrWin.LogLevel}}</code> and above and responses of 500 or higher appear here on their own. A JavaScript error needs one line in the page: <code><script src="/__carlos/carlos.js" defer></script></code> — messages, stacks and fields are redacted for addresses, credentials and long tokens before they leave the page and again on arrival (a floor, not a guarantee). A server-side error needs the hook (next step). The level is set on the Settings tab. The same view from a terminal: <code>carlos errors --app {{.AppName}}</code>.</p> |
| {{end}} |
| {{template "bottom" .}}{{end}} |