feat(analytics): capture valid and invalid visit outcomes #13

Merged
eleith merged 1 commits from feat/visit-outcomes into main 2026-11-05 23:09:06 +00:00
17 changed files with 231 additions and 71 deletions

No files matched your search

+6
View File
@@ -0,0 +1,6 @@
---
'@goto/web': minor
'@goto/worker': minor
---
Record request-time valid and invalid visit outcomes for known redirect domains, including base URLs and inactive or unmatched paths. Preserve existing visits as valid without consulting current YAML, while keeping HEAD, malformed requests, unknown hosts, and disabled analytics untracked.
+19 -3
View File
@@ -18,7 +18,7 @@ function visitCount(path: string, logicalPath: string): number {
}
}
test('the compiled writer isolates lock waits and drains accepted visits before shutdown', async ({
test('the compiled writer persists outcomes, isolates locks and drains visits before shutdown', async ({
request,
appServer
}) => {
@@ -55,13 +55,29 @@ test('the compiled writer isolates lock waits and drains accepted visits before
competingWriter.exec('ROLLBACK');
competingWriter.close();
}
const invalid = await request.get('/missing', options);
const base = await request.get('/', options);
const head = await request.head('/missing', options);
expect(invalid.status()).toBe(404);
expect(base.status()).toBe(404);
expect(head.status()).toBe(404);
expect(await appServer.stop()).toEqual({ code: 0, signal: null });
expect(visitCount(databasePath, '/docs')).toBe(3);
const db = new DatabaseSync(databasePath, { readOnly: true });
try {
expect(
db.prepare('SELECT DISTINCT browser, os FROM visits WHERE path = ?').all('/docs')
).toEqual([{ browser: 'Firefox', os: 'Linux' }]);
db.prepare('SELECT DISTINCT outcome, browser, os FROM visits WHERE path = ?').all('/docs')
).toEqual([{ outcome: 'valid', browser: 'Firefox', os: 'Linux' }]);
expect(
db
.prepare(
'SELECT path, outcome FROM visits WHERE domain = ? AND path IN (?, ?) ORDER BY path'
)
.all('go.example.org', '/', '/missing')
).toEqual([
{ path: '/', outcome: 'invalid' },
{ path: '/missing', outcome: 'invalid' }
]);
} finally {
db.close();
}
+29 -3
View File
@@ -101,7 +101,7 @@ test('rejects malformed ingress without reaching either handler', () => {
expect(requestApplication(incoming)).toBeUndefined();
});
test('captures only raw visit fields from the matched logical path and trusted IP header', () => {
test('captures the request outcome, logical path and raw fields using the trusted IP header', () => {
const recordVisit = vi.fn();
const tracked = { ...application, recordVisit };
const incoming = request('go.example.test', '/%73chema.json?private=not-collected');
@@ -117,6 +117,7 @@ test('captures only raw visit fields from the matched logical path and trusted I
expect(recordVisit).toHaveBeenCalledExactlyOnceWith({
domain: 'go.example.test',
path: '/schema.json',
outcome: 'valid',
visited_at: expect.any(Number),
ip: '2001:db8::1',
user_agent: 'Example agent',
@@ -124,7 +125,7 @@ test('captures only raw visit fields from the matched logical path and trusted I
});
});
test('does not record HEAD, fallback redirects, or visits when analytics is disabled', () => {
test('captures invalid decoded paths even when the fallback redirects', () => {
const recordVisit = vi.fn();
const fallbackSnapshot = {
...snapshot,
@@ -137,10 +138,28 @@ test('does not record HEAD, fallback redirects, or visits when analytics is disa
}
};
const tracked = { currentSnapshot: () => fallbackSnapshot, recordVisit };
const incoming = request('go.example.test', '/%6dissing?private=not-collected');
expect(handleIngress(tracked, incoming, {})?.status).toBe(302);
expect(recordVisit).toHaveBeenCalledExactlyOnceWith({
domain: 'go.example.test',
path: '/missing',
outcome: 'invalid',
visited_at: expect.any(Number),
ip: '127.0.0.1',
user_agent: null,
accept_language: null
});
});
test('does not record HEAD, malformed, admin, unknown-host or disabled-analytics requests', () => {
const recordVisit = vi.fn();
const tracked = { ...application, recordVisit };
const head = request('go.example.test');
head.method = 'HEAD';
expect(handleIngress(tracked, head, {})?.status).toBe(302);
expect(handleIngress(tracked, request('go.example.test', '/missing'), {})?.status).toBe(302);
expect(handleIngress(tracked, request('go.example.test', '/%'), {})?.status).toBe(400);
expect(handleIngress(tracked, request('unknown.example.test'), {})?.status).toBe(404);
expect(handleIngress(tracked, request('admin.example.test'), {})).toBeUndefined();
const disabled = {
...snapshot,
configuration: {
@@ -152,6 +171,13 @@ test('does not record HEAD, fallback redirects, or visits when analytics is disa
handleIngress({ currentSnapshot: () => disabled, recordVisit }, request('go.example.test'), {})
?.status
).toBe(302);
expect(
handleIngress(
{ currentSnapshot: () => disabled, recordVisit },
request('go.example.test', '/missing'),
{}
)?.status
).toBe(404);
expect(recordVisit).not.toHaveBeenCalled();
});
+2 -2
View File
@@ -3,7 +3,7 @@ import type { Application } from './application.js';
import { isLocalProbe, readIngress } from './ingress.js';
import { resolveRedirect } from './redirect.js';
import { bindApplication } from './request-context.js';
import { recordRedirectVisit } from './visits/capture.js';
import { captureVisit } from './visits/capture.js';
function response(status: number): Response {
return new Response(null, { status, headers: { 'cache-control': 'no-store' } });
@@ -26,7 +26,7 @@ export function handleIngress(
request.method ?? 'GET'
);
if (snapshot.configuration.analytics.retention_months !== 0) {
recordRedirectVisit(request, redirect.visit, application.recordVisit);
captureVisit(request, redirect.visit, application.recordVisit);
}
return redirect.response;
}
+46 -25
View File
@@ -55,10 +55,11 @@ describe('exact single-decode lookup', () => {
['/100%25', '/100%'],
['/docs?next=/missing?part=two', '/docs']
])('%s resolves logical key %s', (target, path) => {
const { response } = resolveRedirect(domain, host, target, 'GET');
const { response, visit } = resolveRedirect(domain, host, target, 'GET');
expect(response.status).toBe(302);
expect(response.headers.get('location')).toBe(domain.links[path]?.url);
expect(visit).toMatchObject({ path, outcome: 'valid' });
});
test.each([
@@ -71,9 +72,10 @@ describe('exact single-decode lookup', () => {
'/docs%3Fignored=one',
'/docs%23fragment'
])('%s is not normalized, prefix-matched or reparsed', (target) => {
const { response } = resolveRedirect(domain, host, target, 'GET');
const { response, visit } = resolveRedirect(domain, host, target, 'GET');
expect(response.status).toBe(404);
expect(visit?.outcome).toBe('invalid');
expect(response.headers.has('location')).toBe(false);
expect(response.headers.get('cache-control')).toBe('no-store');
});
@@ -131,9 +133,10 @@ describe('HTTP responses', () => {
'rejects %s without resolving a link or fallback',
(method) => {
const domain = preparedDomain({ '/docs': preparedLink() }, fallback);
const { response } = resolveRedirect(domain, host, '/docs', method);
const { response, visit } = resolveRedirect(domain, host, '/docs', method);
expect(response.status).toBe(405);
expect(visit).toBeUndefined();
expect(response.headers.get('allow')).toBe('GET, HEAD');
expect(response.headers.get('cache-control')).toBe('no-store');
expect(response.headers.has('location')).toBe(false);
@@ -161,9 +164,11 @@ describe('HTTP responses', () => {
'/%C3%28',
'/%FF'
])('rejects malformed raw target %j before using fallback', (target) => {
const { response } = resolveRedirect(preparedDomain({}, fallback), host, target, 'GET');
const domain = preparedDomain({}, fallback);
const { response, visit } = resolveRedirect(domain, host, target, 'GET');
expect(response.status).toBe(400);
expect(visit).toBeUndefined();
expect(response.headers.get('cache-control')).toBe('no-store');
expect(response.headers.has('location')).toBe(false);
});
@@ -197,13 +202,13 @@ describe('validity and fallback', () => {
const end = Date.parse(until);
test.each([
{ enabled: true, now: start - 1, status: 302, location: fallback },
{ enabled: true, now: start, status: 307, location: destination },
{ enabled: true, now: end - 1, status: 307, location: destination },
{ enabled: true, now: end, status: 302, location: fallback },
{ enabled: true, now: end + 1, status: 302, location: fallback },
{ enabled: false, now: start, status: 302, location: fallback }
])('enabled=$enabled at $now returns $status', ({ enabled, now, status, location }) => {
{ enabled: true, now: start - 1, status: 302, location: fallback, outcome: 'invalid' },
{ enabled: true, now: start, status: 307, location: destination, outcome: 'valid' },
{ enabled: true, now: end - 1, status: 307, location: destination, outcome: 'valid' },
{ enabled: true, now: end, status: 302, location: fallback, outcome: 'invalid' },
{ enabled: true, now: end + 1, status: 302, location: fallback, outcome: 'invalid' },
{ enabled: false, now: start, status: 302, location: fallback, outcome: 'invalid' }
])('enabled=$enabled at $now returns $status', ({ enabled, now, status, location, outcome }) => {
const domain = preparedDomain(
{
'/docs': preparedLink({
@@ -215,13 +220,24 @@ describe('validity and fallback', () => {
},
fallback
);
const { response } = resolveRedirect(domain, host, '/docs?incoming=ignored', 'GET', now);
const { response, visit } = resolveRedirect(domain, host, '/docs?incoming=ignored', 'GET', now);
expect(response.status).toBe(status);
expect(visit).toEqual({ domain: host, path: '/docs', outcome, visitedAt: now });
expect(response.headers.get('location')).toBe(location);
expect(response.headers.get('cache-control')).toBe('no-store');
});
test('visit outcomes do not change with later link configuration', () => {
const link = preparedLink();
const domain = preparedDomain({ '/docs': link }, fallback);
const first = resolveRedirect(domain, host, '/docs', 'GET');
link.enabled = false;
const next = resolveRedirect(domain, host, '/docs', 'GET');
expect(first.visit?.outcome).toBe('valid');
expect(next.visit?.outcome).toBe('invalid');
});
test('uses Date.now when no clock is supplied', () => {
const clock = vi.spyOn(Date, 'now').mockReturnValue(start);
try {
@@ -229,7 +245,7 @@ describe('validity and fallback', () => {
const { response, visit } = resolveRedirect(domain, host, '/docs', 'GET');
expect(response.status).toBe(302);
expect(visit).toEqual({ domain: host, path: '/docs', visitedAt: start });
expect(visit).toEqual({ domain: host, path: '/docs', outcome: 'valid', visitedAt: start });
} finally {
clock.mockRestore();
}
@@ -238,15 +254,11 @@ describe('validity and fallback', () => {
test.each([preparedLink({ enabled: false }), preparedLink({ valid_until: until })])(
'inactive links can fall back to 404',
(link) => {
const { response } = resolveRedirect(
preparedDomain({ '/docs': link }),
host,
'/docs',
'GET',
end
);
const domain = preparedDomain({ '/docs': link });
const { response, visit } = resolveRedirect(domain, host, '/docs', 'GET', end);
expect(response.status).toBe(404);
expect(visit?.outcome).toBe('invalid');
expect(response.headers.has('location')).toBe(false);
expect(response.headers.get('cache-control')).toBe('no-store');
}
@@ -255,20 +267,29 @@ describe('validity and fallback', () => {
test('a root link takes precedence over URL fallback', () => {
const domain = preparedDomain({ '/': preparedLink() }, fallback);
expect(
resolveRedirect(domain, host, '/?incoming=ignored', 'GET').response.headers.get('location')
).toBe(destination);
const { response, visit } = resolveRedirect(domain, host, '/?incoming=ignored', 'GET');
expect(response.headers.get('location')).toBe(destination);
expect(visit).toMatchObject({ path: '/', outcome: 'valid' });
});
test('an unconfigured root remains a base visit with an invalid outcome', () => {
const domain = preparedDomain();
const { response, visit } = resolveRedirect(domain, host, '/', 'GET');
expect(response.status).toBe(404);
expect(visit).toMatchObject({ path: '/', outcome: 'invalid' });
});
test('URL fallback is always 302 and never appends the incoming path or query', () => {
const { response } = resolveRedirect(
preparedDomain({}, fallback),
const domain = preparedDomain({}, fallback);
const { response, visit } = resolveRedirect(
domain,
host,
'/missing/nested?owned=replaced&new=value',
'GET'
);
expect(response.status).toBe(302);
expect(visit).toMatchObject({ path: '/missing/nested', outcome: 'invalid' });
expect(response.headers.get('location')).toBe(fallback);
expect(response.headers.get('cache-control')).toBe('no-store');
});
+85 -27
View File
@@ -1,9 +1,11 @@
import { isSelfRedirect, type PreparedDomain } from '@goto/config';
import { isSelfRedirect, type PreparedDomain, type PreparedLink } from '@goto/config';
import type { VisitOutcome } from '@goto/db';
import type { Immutable } from './snapshot.js';
export interface ResolvedVisit {
domain: string;
path: string;
outcome: VisitOutcome;
visitedAt: number;
}
@@ -12,7 +14,17 @@ export interface RedirectResult {
visit?: ResolvedVisit;
}
function parseTarget(target: string): { path: string; query: string } | undefined {
interface ParsedTarget {
path: string;
query: string;
}
interface ResolvedResponse {
response: Response;
outcome: VisitOutcome;
}
function parseTarget(target: string): ParsedTarget | undefined {
if (!target.startsWith('/') || target.includes('#') || target.includes('\\')) return undefined;
if (!target.isWellFormed()) return undefined;
for (let index = 0; index < target.length; index++) {
@@ -51,6 +63,61 @@ function serializeLocation(destination: string): string {
return destination;
}
function emptyResponse(status: number): Response {
return new Response(null, { status, headers: { 'cache-control': 'no-store' } });
}
function redirectResponse(destination: string, status: number): Response {
const location = serializeLocation(destination);
const headers = { 'cache-control': 'no-store', location };
return new Response(null, { status, headers });
}
function findActiveLink(
domain: Immutable<PreparedDomain>,
path: string,
now: number
): Immutable<PreparedLink> | undefined {
const link = Object.hasOwn(domain.links, path) ? domain.links[path] : undefined;
if (!link || !link.enabled) return;
const started = link.valid_from === undefined || now >= Date.parse(link.valid_from);
const notExpired = link.valid_until === undefined || now < Date.parse(link.valid_until);
if (started && notExpired) return link;
}
function fallbackResponse(
fallback: PreparedDomain['fallback'],
host: string,
path: string
): Response {
if (fallback === 404) return emptyResponse(404);
if (isSelfRedirect(fallback, host, path)) return emptyResponse(404);
return redirectResponse(fallback, 302);
}
function resolveVisitResponse(
domain: Immutable<PreparedDomain>,
host: string,
target: ParsedTarget,
now: number
): ResolvedResponse {
const link = findActiveLink(domain, target.path, now);
if (!link) {
const response = fallbackResponse(domain.fallback, host, target.path);
return { response, outcome: 'invalid' };
}
let destination = link.url;
if (link.forward_query) destination = forwardQuery(link.url, target.query);
if (isSelfRedirect(destination, host, target.path)) {
const response = emptyResponse(404);
return { response, outcome: 'invalid' };
}
const response = redirectResponse(destination, link.status_code);
return { response, outcome: 'valid' };
}
export function resolveRedirect(
domain: Immutable<PreparedDomain>,
host: string,
@@ -58,35 +125,26 @@ export function resolveRedirect(
method: string,
now: number = Date.now()
): RedirectResult {
const headers = new Headers({ 'cache-control': 'no-store' });
if (method !== 'GET' && method !== 'HEAD') {
headers.set('allow', 'GET, HEAD');
return { response: new Response(null, { status: 405, headers }) };
const response = emptyResponse(405);
response.headers.set('allow', 'GET, HEAD');
return { response };
}
const parsed = parseTarget(target);
if (parsed === undefined) return { response: new Response(null, { status: 400, headers }) };
const link = Object.hasOwn(domain.links, parsed.path) ? domain.links[parsed.path] : undefined;
const active =
link !== undefined &&
link.enabled &&
(link.valid_from === undefined || now >= Date.parse(link.valid_from)) &&
(link.valid_until === undefined || now < Date.parse(link.valid_until));
let destination = domain.fallback;
let status = 302;
if (link !== undefined && active) {
destination = link.forward_query ? forwardQuery(link.url, parsed.query) : link.url;
status = link.status_code;
if (parsed === undefined) {
const response = emptyResponse(400);
return { response };
}
if (destination === 404 || isSelfRedirect(destination, host, parsed.path)) {
return { response: new Response(null, { status: 404, headers }) };
}
headers.set('location', serializeLocation(destination));
const response = new Response(null, { status, headers });
if (active && method === 'GET') {
return { response, visit: { domain: host, path: parsed.path, visitedAt: now } };
}
return { response };
const resolved = resolveVisitResponse(domain, host, parsed, now);
const response = resolved.response;
if (method === 'HEAD') return { response };
const visit: ResolvedVisit = {
domain: host,
path: parsed.path,
outcome: resolved.outcome,
visitedAt: now
};
return { response, visit };
}
+2 -1
View File
@@ -3,7 +3,7 @@ import type { IncomingMessage } from 'node:http';
import { readClientAddress } from '../ingress.js';
import type { ResolvedVisit } from '../redirect.js';
export function recordRedirectVisit(
export function captureVisit(
request: IncomingMessage,
visit: ResolvedVisit | undefined,
recordVisit: ((visit: NewVisit) => void) | undefined
@@ -19,6 +19,7 @@ export function recordRedirectVisit(
recordVisit({
domain: visit.domain,
path: visit.path,
outcome: visit.outcome,
visited_at: visit.visitedAt,
ip,
user_agent: userAgent,
@@ -13,6 +13,7 @@ let writes: ReturnType<typeof Promise.withResolvers<void>>[];
const visit: NewVisit = {
domain: 'go.example.test',
path: '/docs',
outcome: 'valid',
visited_at: 1_700_000_000_000,
ip: '2001:db8::1',
user_agent: null,
+8 -1
View File
@@ -21,7 +21,14 @@ export function createVisitRecorder(databasePath: string): VisitRecorder {
}
function exceedsVisitSizeLimit(visit: NewVisit): boolean {
const textFields = [visit.domain, visit.path, visit.ip, visit.user_agent, visit.accept_language];
const textFields = [
visit.domain,
visit.path,
visit.outcome,
visit.ip,
visit.user_agent,
visit.accept_language
];
let bytes = 8;
for (const value of textFields) {
bytes += Buffer.byteLength(value ?? '');
+1
View File
@@ -13,6 +13,7 @@ import { insertVisit, openDb, runMigrations } from '@goto/db';
const visit = {
domain: 'go.example.test',
path: '/docs',
outcome: 'valid',
visited_at: Date.now(),
ip: '127.0.0.1',
user_agent: null,
+3 -2
View File
@@ -1,5 +1,5 @@
import { validateConfig } from '@goto/config';
import { openDb, runMigrations } from '@goto/db';
import { openDb, runMigrations, type ClassifiedVisit } from '@goto/db';
import type { OpenWorkflow } from 'openworkflow';
import { expect, test, vi } from 'vitest';
import {
@@ -41,9 +41,10 @@ function client() {
return { engine, result };
}
const visit = {
const visit: ClassifiedVisit = {
domain: 'go.example.test',
path: '/docs',
outcome: 'valid',
visited_at: 1,
ip: '127.0.0.1',
user_agent: null,
+1
View File
@@ -70,6 +70,7 @@ test('an existing zero-policy database is purged by a real startup workflow', as
await insertVisit(db, {
domain: 'go.example.test',
path: '/docs',
outcome: 'valid',
visited_at: Date.now(),
ip: '127.0.0.1',
user_agent: null,
+1 -1
View File
@@ -7,7 +7,7 @@ import type { Database } from './types.js';
export { runMigrations } from './migrate.js';
export { insertVisit, pruneVisits, purgeVisits } from './visits.js';
export type { ClassifiedVisit, Database, NewVisit, Visit } from './types.js';
export type { ClassifiedVisit, Database, NewVisit, Visit, VisitOutcome } from './types.js';
export async function openDb(
path: string,
@@ -0,0 +1,10 @@
import { sql, type Kysely, type Migration } from 'kysely';
export const addVisitOutcome: Migration = {
async up(db: Kysely<unknown>): Promise<void> {
await sql`ALTER TABLE visits ADD COLUMN outcome text NOT NULL DEFAULT 'valid'`.execute(db);
},
async down(db: Kysely<unknown>): Promise<void> {
await sql`ALTER TABLE visits DROP COLUMN outcome`.execute(db);
}
};
+3 -1
View File
@@ -2,10 +2,12 @@ import { initial } from './0001_visits.js';
import { addVisitClassification } from './0002_visit_classification.js';
import { visitRetention } from './0003_visit_retention.js';
import { removeVisitCollection } from './0004_remove_visit_collection.js';
import { addVisitOutcome } from './0005_visit_outcome.js';
export const migrations = {
'0001_visits': initial,
'0002_visit_classification': addVisitClassification,
'0003_visit_retention': visitRetention,
'0004_remove_visit_collection': removeVisitCollection
'0004_remove_visit_collection': removeVisitCollection,
'0005_visit_outcome': addVisitOutcome
};
+11 -5
View File
@@ -28,6 +28,7 @@ afterEach(async () => {
const visit: ClassifiedVisit = {
domain: 'go.example.test',
path: '/café',
outcome: 'valid',
visited_at: 1_700_000_000_000,
ip: '2001:db8::1',
user_agent: 'Mozilla/5.0 (X11; Linux x86_64; rv:158.0) Gecko/20100101 Firefox/158.0',
@@ -52,7 +53,8 @@ test('opens a real WAL database and applies versioned migrations once', async ()
'0001_visits',
'0002_visit_classification',
'0003_visit_retention',
'0004_remove_visit_collection'
'0004_remove_visit_collection',
'0005_visit_outcome'
]);
const tables = await db.introspection.getTables();
const names = tables.map((table) => table.name);
@@ -63,7 +65,7 @@ test('opens a real WAL database and applies versioned migrations once', async ()
}
});
test('persists classified visits for the same logical path on different domains', async () => {
test('persists request-time outcomes for the same logical path on different domains', async () => {
const path = join(directory, 'visits.sqlite');
const writer = await openDb(path);
const ids: number[] = [];
@@ -74,6 +76,7 @@ test('persists classified visits for the same logical path on different domains'
await insertVisit(writer, {
...visit,
domain: 'tiny.example.test',
outcome: 'invalid',
user_agent: null,
accept_language: null,
browser: 'Other',
@@ -91,6 +94,7 @@ test('persists classified visits for the same logical path on different domains'
{
...visit,
domain: 'tiny.example.test',
outcome: 'invalid',
id: ids[1],
user_agent: null,
accept_language: null,
@@ -104,7 +108,7 @@ test('persists classified visits for the same logical path on different domains'
}
});
test('adds classification columns without changing existing visit data', async () => {
test('adds request-time labels without changing existing visit data', async () => {
const db = await openDb(join(directory, 'legacy.sqlite'));
try {
const migrator = new Migrator({
@@ -125,7 +129,8 @@ test('adds classification columns without changing existing visit data', async (
expect(await runMigrations(db)).toEqual([
'0002_visit_classification',
'0003_visit_retention',
'0004_remove_visit_collection'
'0004_remove_visit_collection',
'0005_visit_outcome'
]);
expect(await db.selectFrom('visits').selectAll().executeTakeFirstOrThrow()).toEqual({
...visit,
@@ -162,7 +167,8 @@ test('a failed migration rolls back schema and bookkeeping so startup can retry'
'0001_visits',
'0002_visit_classification',
'0003_visit_retention',
'0004_remove_visit_collection'
'0004_remove_visit_collection',
'0005_visit_outcome'
]);
expect(await runMigrations(db)).toEqual([]);
} finally {
+3
View File
@@ -1,9 +1,12 @@
import type { Generated, Insertable, Selectable } from 'kysely';
export type VisitOutcome = 'valid' | 'invalid';
export interface VisitTable {
id: Generated<number>;
domain: string;
path: string;
outcome: VisitOutcome;
visited_at: number;
ip: string;
user_agent: string | null;