diff --git a/app/(app)/dashboard/ads/earnings/page.tsx b/app/(app)/dashboard/ads/earnings/page.tsx index 54f435d..ac8620a 100644 --- a/app/(app)/dashboard/ads/earnings/page.tsx +++ b/app/(app)/dashboard/ads/earnings/page.tsx @@ -3,7 +3,7 @@ import dynamic from "next/dynamic"; import { createClient } from "@/lib/supabase/server"; import { loadEarnings, dollars, type EarningsModel } from "@/lib/ads/earnings-data"; import { EarningsPdfButton } from "@/components/ads/earnings-pdf-button"; -import { StatsUnavailable } from "@/components/ads/stats-unavailable"; +import { StatsUnavailable } from "@/components/stats-unavailable"; // recharts is client-only; keep it out of the server bundle. const MoneyTrend = dynamic(() => import("@/components/ads/money-trend").then((m) => m.MoneyTrend)); diff --git a/app/(app)/dashboard/ads/page.tsx b/app/(app)/dashboard/ads/page.tsx index b8864d8..a5a3615 100644 --- a/app/(app)/dashboard/ads/page.tsx +++ b/app/(app)/dashboard/ads/page.tsx @@ -6,7 +6,7 @@ import { MiniTrend } from "@/components/ads/mini-trend"; import { AccountTrend } from "@/components/ads/account-trend"; import { RangeTabs } from "@/components/ads/range-tabs"; import { StatSpark } from "@/components/ads/stat-spark"; -import { StatsUnavailable } from "@/components/ads/stats-unavailable"; +import { StatsUnavailable } from "@/components/stats-unavailable"; import { deliveredClicks, deliveredImpressions, diff --git a/app/(app)/dashboard/ads/slots/page.tsx b/app/(app)/dashboard/ads/slots/page.tsx index b7cef39..7fcdbe9 100644 --- a/app/(app)/dashboard/ads/slots/page.tsx +++ b/app/(app)/dashboard/ads/slots/page.tsx @@ -3,7 +3,7 @@ import { createClient } from "@/lib/supabase/server"; import { env } from "@/lib/env"; import { fetchSupportedTokens } from "@/lib/coinpay-tokens"; import { SlotManager } from "@/components/ads/slot-manager"; -import { StatsUnavailable } from "@/components/ads/stats-unavailable"; +import { StatsUnavailable } from "@/components/stats-unavailable"; import { deliveredClicks, deliveredImpressions, diff --git a/app/(app)/dashboard/analytics/page.tsx b/app/(app)/dashboard/analytics/page.tsx index 2c9b610..e358295 100644 --- a/app/(app)/dashboard/analytics/page.tsx +++ b/app/(app)/dashboard/analytics/page.tsx @@ -1,5 +1,7 @@ import Link from "next/link"; import { createClient } from "@/lib/supabase/server"; +import { rpcFailed } from "@/lib/loaded"; +import { StatsUnavailable } from "@/components/stats-unavailable"; import { ProjectLogo } from "@/components/project-logo"; import { FontSparkline } from "@/components/font-sparkline"; import { bucketLabel } from "@/lib/tracker/categorize"; @@ -175,6 +177,30 @@ export default async function PortfolioAnalyticsPage({ supabase.rpc("tracker_device_totals_multi", { p_projects: projectIds, days }), ]); + // Every panel below zero-fills on failure so one bad query cannot take the + // page down. Without this flag that is indistinguishable from a portfolio + // with no traffic: these eleven RPCs run concurrently over every project the + // account owns, and when they were cancelled by the 8s statement_timeout the + // whole page rendered as zeros with nothing logged. + const rpcResults: Array<[string, { error: { message?: string } | null }]> = [ + ["tracker_daily_series_multi", seriesRes], + ["tracker_project_totals", totalsRes], + ["tracker_bucket_totals_multi", bucketsRes], + ["tracker_event_mix_multi", mixRes], + ["tracker_top_pages_multi", pagesRes], + ["tracker_top_referrers_multi", referrersRes], + ["tracker_top_actions_multi", actionsRes], + ["tracker_top_exit_pages_multi", exitRes], + ["tracker_top_countries_multi", countriesRes], + ["tracker_top_cities_multi", citiesRes], + ["tracker_device_totals_multi", devicesRes], + ]; + let statsFailed = false; + // Not `.some()`: every failure should be logged, not just the first. + for (const [name, res] of rpcResults) { + if (rpcFailed("tracker", name, res.error)) statsFailed = true; + } + const series = ((seriesRes.data ?? []) as Parameters[0][]).map( toSeriesRow, ); @@ -215,10 +241,15 @@ export default async function PortfolioAnalyticsPage({ Math.min(ranked.length, Math.floor(DAILY_ROW_BUDGET / days)), ); const detailIds = ranked.slice(0, detailCount).map((r) => r.project.id); - const { data: perProjectRaw } = await supabase.rpc( + const { data: perProjectRaw, error: perProjectError } = await supabase.rpc( "tracker_project_daily_series", { p_projects: detailIds, days }, ); + const perProjectFailed = rpcFailed( + "tracker", + "tracker_project_daily_series", + perProjectError, + ); const axis = utcDayAxis(days); const dailyByProject = new Map>(); @@ -397,6 +428,10 @@ export default async function PortfolioAnalyticsPage({
+ {(statsFailed || perProjectFailed) && ( + + )} +

{verdict}

diff --git a/app/(app)/dashboard/page.tsx b/app/(app)/dashboard/page.tsx index 99fd295..d81b847 100644 --- a/app/(app)/dashboard/page.tsx +++ b/app/(app)/dashboard/page.tsx @@ -1,8 +1,10 @@ import Link from "next/link"; import { createClient } from "@/lib/supabase/server"; +import { rpcFailed, type Loaded } from "@/lib/loaded"; import { ScoreBadge } from "@/components/score-badge"; import { FontSparkline } from "@/components/font-sparkline"; import { ProjectLogo } from "@/components/project-logo"; +import { StatsUnavailable } from "@/components/stats-unavailable"; import { backfillProjectLogo } from "@/app/actions/createProject"; import { getOrCreateDefaultOrg, isOrgWideRole, listUserOrgs, missingOrgSchema } from "@/lib/orgs"; import { listOrgTeam } from "@/app/actions/org-members"; @@ -118,12 +120,14 @@ export default async function DashboardPage({ // project has an lx_site row in status=active; social is "on" when at // least one social account is linked at the project level. const projectIds = (projects ?? []).map((p) => p.id); - const [autoblogIds, socialIds, latestPosts, trafficByProject] = await Promise.all([ + const [autoblogIds, socialIds, latestPosts, traffic] = await Promise.all([ fetchEnabledProjectIds(supabase, "lx_site", projectIds, { status: "active" }), fetchEnabledProjectIds(supabase, "sp_site_account", projectIds), fetchLatestBlogPostByProject(supabase, projectIds), fetchSevenDayPageviews(supabase, projectIds), ]); + const trafficByProject = traffic.data; + const trafficFailed = traffic.failed; // Lazy backfill: any project still missing a logo gets one scraped // in the background on this dashboard hit. Fire-and-forget — the @@ -187,6 +191,10 @@ export default async function DashboardPage({

+ {projects && projects.length > 0 && trafficFailed && ( + + )} + {projects && projects.length > 0 ? (
    {projects.map((p) => ( @@ -256,13 +264,17 @@ export default async function DashboardPage({
    - {totalTraffic(trafficByProject.get(p.id) ?? []).toLocaleString()} pageviews + {trafficFailed + ? "Pageviews unavailable" + : `${totalTraffic(trafficByProject.get(p.id) ?? []).toLocaleString()} pageviews`}
    - Past 7 days + {trafficFailed ? "Query failed \u2014 not zero" : "Past 7 days"}
    - + {!trafficFailed && ( + + )}
    {orgSchemaReady && ( >, projectIds: string[], -): Promise> { +): Promise>> { const days = lastSevenDays(); const out = new Map(); for (const projectId of projectIds) { @@ -372,15 +384,24 @@ async function fetchSevenDayPageviews( days.map((day) => ({ day, count: 0 })), ); } - if (projectIds.length === 0) return out; + if (projectIds.length === 0) return { data: out, failed: false }; // Server-side aggregator — selecting raw rows runs into PostgREST's // 1000-row response cap and silently truncates whichever projects' // rows didn't make the cut. - const { data } = await supabase.rpc("dashboard_project_pageviews", { + // + // The error is checked rather than discarded: this RPC reads a 1.2M-row + // rollup under RLS, and when it was cancelled by the 8s statement_timeout + // the zero-filled map below rendered "0 pageviews" on every card. That is + // indistinguishable from a portfolio with no traffic, and was read as a + // dead tracker three times over. + const { data, error } = await supabase.rpc("dashboard_project_pageviews", { p_project_ids: projectIds, p_since: days[0], }); + if (rpcFailed("tracker", "dashboard_project_pageviews", error)) { + return { data: out, failed: true }; + } for (const row of (data ?? []) as Array<{ project_id: string; @@ -393,7 +414,7 @@ async function fetchSevenDayPageviews( if (point) point.count += Number(row.count); } - return out; + return { data: out, failed: false }; } function lastSevenDays() { diff --git a/components/ads/stats-unavailable.tsx b/components/stats-unavailable.tsx similarity index 61% rename from components/ads/stats-unavailable.tsx rename to components/stats-unavailable.tsx index a8c1688..ad14e54 100644 --- a/components/ads/stats-unavailable.tsx +++ b/components/stats-unavailable.tsx @@ -2,10 +2,12 @@ * Shown when a stats query failed, in place of the zeros it would otherwise * have rendered. * - * The ad dashboards zero-fill on failure so one bad panel cannot take the page - * down. Without this banner that choice is indistinguishable from a real run of - * no delivery, which is how a live network reporting six figures of impressions - * came to show four zeros and read as a dead pipeline. + * The ad and tracker dashboards both zero-fill on failure so one bad panel + * cannot take the page down. Without this banner that choice is + * indistinguishable from a real run of no traffic, which is how a live network + * reporting six figures of impressions came to show four zeros, and how every + * project on the portfolio page came to read 0 pageviews while ingest was + * writing rows every second. */ export function StatsUnavailable({ what = "these figures" }: { what?: string }) { return ( diff --git a/lib/ads/series.ts b/lib/ads/series.ts index 30a460e..6b43003 100644 --- a/lib/ads/series.ts +++ b/lib/ads/series.ts @@ -1,5 +1,6 @@ import type { SupabaseClient } from "@supabase/supabase-js"; import { bucketAxis, bucketOf, rangeSince, type RangeDef } from "./ranges"; +import { rpcFailed as rpcFailedIn, type Loaded } from "@/lib/loaded"; export type AccountPoint = { /** Bucket start, epoch ms (epoch-aligned, matching SQL date_bin). */ @@ -27,34 +28,15 @@ export const EMPTY_TOTALS: RangeTotals = { spentCents: 0, }; -/** - * A loader's result together with whether the query behind it actually ran. - * - * Every loader below zero-fills rather than throwing, so one failing panel - * cannot take the whole page down with it. That part is deliberate and stays. - * What it cost is a way to tell the two apart: a cancelled query and a - * genuinely quiet range produced byte-identical output, so the dashboard could - * report four zeros over a network delivering six figures and say nothing was - * wrong. It has now done exactly that three times, for three unrelated reasons - * — the paid-only measure (#199), the same bug on two more surfaces (#225), and - * an RPC being cancelled by statement_timeout. Each time the zeros were read as - * a dead pipeline and diagnosed from scratch. - * - * `failed` is how a caller tells "we could not load this" from "this is 0". - */ -export type Loaded = { data: T; failed: boolean }; +// `Loaded` and the failure logging live in lib/loaded.ts now: the tracker +// dashboards swallowed their RPC errors exactly the same way and reported 0 +// pageviews across every project while ingest was healthy, so the two halves +// of the product share one helper rather than each growing their own. + +export type { Loaded }; -/** - * Record an RPC failure and report whether there was one. - * - * Logs, because the error was previously discarded at the point of failure: - * the only surviving evidence that a query had been cancelled was in Postgres' - * own logs, which is a long way to go to explain a tile reading 0. - */ function rpcFailed(name: string, error: { message?: string } | null): boolean { - if (!error) return false; - console.error(`[ads] RPC ${name} failed: ${error.message ?? "unknown error"}`); - return true; + return rpcFailedIn("ads", name, error); } type AccountSeriesRow = { diff --git a/lib/loaded.ts b/lib/loaded.ts new file mode 100644 index 0000000..9f7dd8d --- /dev/null +++ b/lib/loaded.ts @@ -0,0 +1,37 @@ +/** + * A loader's result together with whether the query behind it actually ran. + * + * Every loader that uses this zero-fills rather than throwing, so one failing + * panel cannot take the whole page down with it. That part is deliberate and + * stays. What it cost is a way to tell the two apart: a cancelled query and a + * genuinely quiet range produced byte-identical output, so a dashboard could + * report zeros over a live pipeline and say nothing was wrong. + * + * That has now happened on both halves of the product. On the ad surfaces it + * took four goes to pin down — the paid-only measure (#199), the same bug on + * two more surfaces (#225), and RPCs cancelled by statement_timeout (#226). + * The tracker surfaces swallowed their errors the same way and reported 0 + * pageviews for every project while ingest was writing rows every second. + * + * `failed` is how a caller tells "we could not load this" from "this is 0". + */ +export type Loaded = { data: T; failed: boolean }; + +/** + * Record an RPC failure and report whether there was one. + * + * Logs, because the error was previously discarded at the point of failure: + * the only surviving evidence that a query had been cancelled was in Postgres' + * own logs, which is a long way to go to explain a tile reading 0. + */ +export function rpcFailed( + scope: string, + name: string, + error: { message?: string } | null, +): boolean { + if (!error) return false; + console.error( + `[${scope}] RPC ${name} failed: ${error.message ?? "unknown error"}`, + ); + return true; +} diff --git a/supabase/migrations/20260902120000_tracker_reporting_indexes.sql b/supabase/migrations/20260902120000_tracker_reporting_indexes.sql new file mode 100644 index 0000000..26d8452 --- /dev/null +++ b/supabase/migrations/20260902120000_tracker_reporting_indexes.sql @@ -0,0 +1,125 @@ +-- Tracker reporting: cover the rollup reads so they stop hitting the timeout. +-- +-- The portfolio dashboard read "0 pageviews" on every project while ingest was +-- writing a row a second. Same *shape* as the ad dashboard bug fixed in #226, +-- but NOT the same cause -- worth stating plainly, because the obvious move +-- (flip these to security definer, as #226 did) is both wrong and unsafe here. +-- +-- Wrong, because RLS is not what costs: the same query run as `postgres` with +-- no policy in the plan still took 8.4s. Unsafe, because the ad RPCs each +-- authorised themselves (`_id in (select id from owned)`) and these do not +-- -- every tracker_*_multi takes p_projects straight from the caller and leans +-- entirely on RLS to decide what it may read. Made definer as they stand, any +-- authenticated user could pass another account's project ids and read their +-- analytics. If these are ever made definer they must grow an ownership +-- filter of their own first. +-- +-- The actual cause is the index. tracker_event_daily_stats_project_event_idx +-- is (project_id, event) with no `day`, so for the dashboard's +-- "project in (...) and event = 'pageview' and day >= X" the planner matched +-- 373,506 index entries, heap-fetched every one to read `day` and `count`, and +-- threw 228,872 of them away on the filter: 152,894 buffers and 9,401ms for a +-- 260-row answer, against the 8s statement_timeout on `authenticated`. +-- +-- Measured on prod (ref ywcizjsgrcmhgyplldac, 1.21M rows / 326MB), as the +-- 48-project owner, with RLS on: +-- +-- dashboard_project_pageviews 9,401ms / 152,894 buf -> 115ms / 19,647 +-- tracker_top_pages_multi 5,764ms / 77,663 buf -> 752ms / 22,764 +-- tracker_top_actions_multi 1,927ms / 326,441 buf -> 635ms / 41,379 +-- tracker_top_referrers_multi 1,518ms / 328,030 buf -> ~1.7s / 41,358 +-- +-- `page_path` rides in the INCLUDE of the (project_id, event, day) index +-- specifically so tracker_top_pages_multi runs index-only. Without it the +-- planner still picks that index for the event predicate but has to heap-fetch +-- all 373,710 matching rows to read the path -- 111,534 buffers, measured, and +-- worse than before the index existed. +-- +-- Both indexes carry `count` in INCLUDE so the aggregates run index-only. That +-- does cost writes: `count` is now an indexed value, so the per-event upsert +-- can no longer take the HOT path. Ingest is ~1-3 events/sec against 1.2M +-- rows, so this is the right side of the trade, but it is the thing to watch +-- if ingest volume grows an order of magnitude. +-- +-- These were created CONCURRENTLY on prod on 2026-09-02 (a 326MB table under +-- live ingest); `if not exists` here so replay is a no-op rather than a lock. +-- +-- Longer term this is still the rollup problem #226 flagged: these queries +-- aggregate 300k-1.2M rows per page load and indexes only make that cheaper, +-- not small. A pre-aggregated daily table is the next move if it regresses. + +create index if not exists tracker_event_daily_stats_project_event_day_idx + on public.tracker_event_daily_stats (project_id, event, day desc) + include (page_path, count); + +create index if not exists tracker_event_daily_stats_project_day_cover_idx + on public.tracker_event_daily_stats (project_id, day desc) + include (event, page_path, referrer_host, event_target, count); + +-- Superseded: (project_id, event) is a strict prefix of the new +-- (project_id, event, day desc). Dropped rather than left in place because it +-- is not merely redundant, it is the trap -- it is what the planner kept +-- choosing over the day-bearing indexes. +drop index if exists public.tracker_event_daily_stats_project_event_idx; + +-- work_mem is 3.5MB on this instance. The three panels that group by a +-- high-cardinality text column (page_path, event_target) spilled their +-- HashAggregate to disk: tracker_top_pages_multi wrote 4,401 temp blocks and +-- took 5.8s at 365 days, which is inside the 8s ceiling only on a warm cache. +-- Raised per function, not globally, because /dashboard/analytics fires eleven +-- of these concurrently and a session-wide raise multiplies by eleven. +create or replace function public.tracker_top_pages_multi(p_projects uuid[], days integer default 30, lim integer default 10) + returns table(project_id uuid, page_path text, total bigint) + language sql + stable + set search_path to 'public' + set work_mem to '16MB' +as $function$ + select project_id, + coalesce(nullif(page_path, ''), '/') as page_path, + sum(count)::bigint as total + from public.tracker_event_daily_stats + where project_id = any(p_projects) + and event = 'pageview' + and day >= ((now() at time zone 'UTC')::date - (greatest(coalesce(days, 30), 1) - 1)) + group by project_id, 2 + order by total desc + limit greatest(coalesce(lim, 10), 1); +$function$; + +create or replace function public.tracker_top_actions_multi(p_projects uuid[], days integer default 30, lim integer default 10) + returns table(project_id uuid, event text, event_target text, total bigint) + language sql + stable + set search_path to 'public' + set work_mem to '16MB' +as $function$ + select project_id, event, event_target, sum(count)::bigint as total + from public.tracker_event_daily_stats + where project_id = any(p_projects) + and event <> 'pageview' + and coalesce(event_target, '') <> '' + and day >= ((now() at time zone 'UTC')::date - (greatest(coalesce(days, 30), 1) - 1)) + group by project_id, event, event_target + order by total desc + limit greatest(coalesce(lim, 10), 1); +$function$; + +create or replace function public.tracker_top_exit_pages_multi(p_projects uuid[], days integer default 30, lim integer default 10) + returns table(project_id uuid, page_path text, total bigint) + language sql + stable + set search_path to 'public' + set work_mem to '16MB' +as $function$ + select project_id, + coalesce(nullif(page_path, ''), '/') as page_path, + sum(count)::bigint as total + from public.tracker_exit_daily_stats + where project_id = any(p_projects) + and count > 0 + and day >= ((now() at time zone 'UTC')::date - (greatest(coalesce(days, 30), 1) - 1)) + group by project_id, 2 + order by total desc + limit greatest(coalesce(lim, 10), 1); +$function$; diff --git a/tests/tracker-stats-failure.test.ts b/tests/tracker-stats-failure.test.ts new file mode 100644 index 0000000..c7c8270 --- /dev/null +++ b/tests/tracker-stats-failure.test.ts @@ -0,0 +1,67 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { rpcFailed, type Loaded } from "@/lib/loaded"; + +// The portfolio dashboard reported "0 pageviews" on every project while the +// tracker was writing a row a second. dashboard_project_pageviews was being +// cancelled by the 8s statement_timeout and returning HTTP 500, and the loader +// read only `data` -- `const { data } = await supabase.rpc(...)` -- so a +// cancelled query and a genuinely quiet week produced byte-identical output. +// +// This is the fourth time zeros-on-failure has been diagnosed from scratch on +// this product (#199, #225, #226, and this). The point of these tests is that +// "we could not read this" can never again be indistinguishable from "this is 0". + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("rpcFailed", () => { + it("reports no failure when the RPC returned no error", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + expect(rpcFailed("tracker", "dashboard_project_pageviews", null)).toBe(false); + expect(spy).not.toHaveBeenCalled(); + }); + + it("reports a failure and logs it, so the cause is not only in Postgres' logs", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + const failed = rpcFailed("tracker", "dashboard_project_pageviews", { + message: "canceling statement due to statement timeout", + }); + expect(failed).toBe(true); + expect(spy).toHaveBeenCalledTimes(1); + expect(String(spy.mock.calls[0][0])).toContain("dashboard_project_pageviews"); + expect(String(spy.mock.calls[0][0])).toContain("statement timeout"); + }); + + it("still reports a failure when the error carries no message", () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + expect(rpcFailed("tracker", "tracker_top_pages_multi", {})).toBe(true); + }); + + it("scopes the log line, so ad and tracker failures are tellable apart", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + rpcFailed("ads", "ad_campaign_totals", { message: "boom" }); + rpcFailed("tracker", "tracker_event_mix_multi", { message: "boom" }); + expect(String(spy.mock.calls[0][0])).toContain("[ads]"); + expect(String(spy.mock.calls[1][0])).toContain("[tracker]"); + }); +}); + +describe("a zero-filled Loaded result", () => { + // The zero-fill itself is deliberate and stays: one dead panel must not take + // the page down. What `failed` buys is that the renderer can tell the two + // apart and say "not zero -- missing" instead of drawing a confident 0. + const emptied = (failed: boolean): Loaded> => ({ + data: new Map([["project-a", 0]]), + failed, + }); + + it("looks identical in its data whether it failed or was genuinely quiet", () => { + expect(emptied(true).data).toEqual(emptied(false).data); + }); + + it("is only distinguishable by the flag", () => { + expect(emptied(true).failed).toBe(true); + expect(emptied(false).failed).toBe(false); + }); +});