mirror of
https://github.com/stack-auth/stack.git
synced 2026-06-04 21:04:37 +08:00
fix(analytics): address heatmap PR review feedback
- showHeatmap: handle token-creation failure (reset dialog) + call via runAsynchronouslyWithAlert so errors surface (greptile P1, vercel, cubic) - DEVICE_WIDTH_BUCKETS: use Map to avoid prototype-pollution lookup (greptile P2) - dedup formatClickhouseDateTimeParam/parseBoundedDateTime across heatmap routes by exporting from the internal route (greptile P2) - derive heatmap JWT expiry from HEATMAP_TOKEN_TTL_MS (cubic) - only report 'Invalid route regex' for actual ClickHouse regexp errors, via shared isClickhouseRegexpError helper (cubic, both routes) - generic top-elements error text instead of raw err.message (cubic) - dev-tool: add primary-button :hover style to prevent hover flash (cubic) - selector builder: emit the data-test-id attr name actually matched (cubic)
This commit is contained in:
parent
5b56689d67
commit
dbeb70c287
@ -8,6 +8,7 @@ import { HexclaveAssertionError, StatusError, captureError } from "@stackframe/s
|
||||
import {
|
||||
buildClickmapUrlLikePattern,
|
||||
clampClickmapSampling,
|
||||
formatClickhouseDateTimeParam,
|
||||
getClickmapOriginFilter,
|
||||
getClickmapOriginParams,
|
||||
getClickmapRouteFilter,
|
||||
@ -15,6 +16,8 @@ import {
|
||||
getClickmapUserAndReplayFilter,
|
||||
getClickmapViewportFilter,
|
||||
getDeviceViewportBucket,
|
||||
isClickhouseRegexpError,
|
||||
parseBoundedDateTime,
|
||||
} from "../../internal/analytics/heatmap/route";
|
||||
|
||||
const MAX_WINDOW_DAYS = 31;
|
||||
@ -22,18 +25,6 @@ const ONE_DAY_MS = 24 * 60 * 60 * 1000;
|
||||
const ROUTE_LIMIT = 50;
|
||||
const ELEMENTS_CHAIN_LIMIT = 200;
|
||||
|
||||
function formatClickhouseDateTimeParam(date: Date): string {
|
||||
return date.toISOString().slice(0, 19);
|
||||
}
|
||||
|
||||
function parseBoundedDateTime(value: string, name: string): Date {
|
||||
const date = new Date(value);
|
||||
if (!Number.isFinite(date.getTime())) {
|
||||
throw new StatusError(StatusError.BadRequest, `Invalid ${name}`);
|
||||
}
|
||||
return date;
|
||||
}
|
||||
|
||||
export const POST = createSmartRouteHandler({
|
||||
metadata: {
|
||||
summary: "Get page heatmap data",
|
||||
@ -199,7 +190,7 @@ export const POST = createSmartRouteHandler({
|
||||
if (!(error instanceof ClickHouseError)) {
|
||||
throw error;
|
||||
}
|
||||
if (body.route_regex != null && body.route_regex !== "") {
|
||||
if (body.route_regex != null && body.route_regex !== "" && isClickhouseRegexpError(error)) {
|
||||
throw new StatusError(StatusError.BadRequest, "Invalid route regex");
|
||||
}
|
||||
captureError("analytics-heatmap-clickhouse-fallback", new HexclaveAssertionError(
|
||||
|
||||
@ -14,11 +14,11 @@ const LINKED_LIMIT = 25;
|
||||
const ELEMENTS_CHAIN_LIMIT = 100;
|
||||
const ONE_DAY_MS = 24 * 60 * 60 * 1000;
|
||||
|
||||
function formatClickhouseDateTimeParam(date: Date): string {
|
||||
export function formatClickhouseDateTimeParam(date: Date): string {
|
||||
return date.toISOString().slice(0, 19);
|
||||
}
|
||||
|
||||
function parseBoundedDateTime(value: string, name: string): Date {
|
||||
export function parseBoundedDateTime(value: string, name: string): Date {
|
||||
const date = new Date(value);
|
||||
if (!Number.isFinite(date.getTime())) {
|
||||
throw new StatusError(StatusError.BadRequest, `Invalid ${name}`);
|
||||
@ -26,21 +26,29 @@ function parseBoundedDateTime(value: string, name: string): Date {
|
||||
return date;
|
||||
}
|
||||
|
||||
// ClickHouse raises a query-execution error when a user-supplied route regex
|
||||
// fails to compile. Only those errors should be reported as a 400 "Invalid
|
||||
// route regex"; unrelated ClickHouse failures must fall through to the generic
|
||||
// service-unavailable path instead of being misattributed to the regex.
|
||||
export function isClickhouseRegexpError(error: ClickHouseError): boolean {
|
||||
return /regexp|regular expression|cannot compile/i.test(error.message);
|
||||
}
|
||||
|
||||
// Device class buckets — kept as a back-compat shim for callers that still pass
|
||||
// `device`. Internally collapsed into viewport_width_min/max so the MV order key
|
||||
// (which leads with viewport_width) does the work instead of a multiIf scan.
|
||||
const DEVICE_WIDTH_BUCKETS: Record<string, { min: number, max: number }> = {
|
||||
tv: { min: 1920, max: 65535 },
|
||||
widescreen: { min: 1440, max: 1919 },
|
||||
desktop: { min: 1200, max: 1439 },
|
||||
laptop: { min: 1024, max: 1199 },
|
||||
tablet: { min: 768, max: 1023 },
|
||||
mobile: { min: 0, max: 767 },
|
||||
};
|
||||
const DEVICE_WIDTH_BUCKETS = new Map<string, { min: number, max: number }>([
|
||||
["tv", { min: 1920, max: 65535 }],
|
||||
["widescreen", { min: 1440, max: 1919 }],
|
||||
["desktop", { min: 1200, max: 1439 }],
|
||||
["laptop", { min: 1024, max: 1199 }],
|
||||
["tablet", { min: 768, max: 1023 }],
|
||||
["mobile", { min: 0, max: 767 }],
|
||||
]);
|
||||
|
||||
export function getDeviceViewportBucket(device: string | undefined): { min: number, max: number } | null {
|
||||
if (device == null || device === "") return null;
|
||||
return DEVICE_WIDTH_BUCKETS[device] ?? null;
|
||||
return DEVICE_WIDTH_BUCKETS.get(device) ?? null;
|
||||
}
|
||||
|
||||
// Translate a PostHog-style URL pattern with `*` wildcards into a SQL LIKE
|
||||
@ -415,7 +423,12 @@ export const POST = createSmartRouteHandler({
|
||||
if (!(error instanceof ClickHouseError)) {
|
||||
throw error;
|
||||
}
|
||||
if (body.kind === "session_replay_clicks" && body.route_regex != null && body.route_regex !== "") {
|
||||
if (
|
||||
body.kind === "session_replay_clicks" &&
|
||||
body.route_regex != null &&
|
||||
body.route_regex !== "" &&
|
||||
isClickhouseRegexpError(error)
|
||||
) {
|
||||
throw new StatusError(StatusError.BadRequest, "Invalid route regex");
|
||||
}
|
||||
captureError("internal-analytics-heatmap-clickhouse-fallback", new HexclaveAssertionError(
|
||||
|
||||
@ -59,7 +59,7 @@ export async function createAnalyticsHeatmapToken(options: {
|
||||
const token = await signJWT({
|
||||
issuer: HEATMAP_TOKEN_ISSUER,
|
||||
audience: HEATMAP_TOKEN_AUDIENCE,
|
||||
expirationTime: "24h",
|
||||
expirationTime: `${HEATMAP_TOKEN_TTL_MS / 1000}s`,
|
||||
payload: {
|
||||
kind: HEATMAP_TOKEN_KIND,
|
||||
scope: HEATMAP_TOKEN_SCOPE,
|
||||
|
||||
@ -33,6 +33,7 @@ import {
|
||||
} from "@stackframe/dashboard-ui-components";
|
||||
import type { AnalyticsHeatmapDevice, AnalyticsHeatmapResponse, AnalyticsHeatmapTokenResponse } from "@stackframe/stack-shared/dist/interface/admin-metrics";
|
||||
import { typedEntries } from "@stackframe/stack-shared/dist/utils/objects";
|
||||
import { runAsynchronouslyWithAlert } from "@stackframe/stack-shared/dist/utils/promises";
|
||||
import { stringCompare } from "@stackframe/stack-shared/dist/utils/strings";
|
||||
import { ArrowRight, GlobeHemisphereWest } from "@phosphor-icons/react";
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
@ -163,7 +164,8 @@ function TopElementsPreview(props: {
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
if (cancelled) return;
|
||||
setError(err instanceof Error ? err.message : "Failed to load top elements.");
|
||||
// Avoid surfacing raw error messages to users; show a safe generic message.
|
||||
setError("Failed to load top elements.");
|
||||
setData(null);
|
||||
})
|
||||
.finally(() => {
|
||||
@ -406,7 +408,17 @@ export default function PageClient() {
|
||||
setSelectedOrigin(origin);
|
||||
setToken(null);
|
||||
setDialogOpen(true);
|
||||
const created = await adminApp.createAnalyticsHeatmapToken({ origin: origin.origin });
|
||||
let created: AnalyticsHeatmapTokenResponse;
|
||||
try {
|
||||
created = await adminApp.createAnalyticsHeatmapToken({ origin: origin.origin });
|
||||
} catch (error) {
|
||||
// Token creation failed (network error, expired session, invalid origin,
|
||||
// etc.); close the dialog so it doesn't hang on "Creating..." and let
|
||||
// runAsynchronouslyWithAlert surface the error to the user.
|
||||
setToken(null);
|
||||
setDialogOpen(false);
|
||||
throw error;
|
||||
}
|
||||
setToken(created);
|
||||
const installedInCurrentTab = installHeatmapTokenForCurrentOrigin(created, adminApp.projectId);
|
||||
try {
|
||||
@ -435,7 +447,7 @@ export default function PageClient() {
|
||||
</Typography>
|
||||
<Input value={customOrigin} onChange={(event) => setCustomOrigin(event.target.value)} placeholder="http://localhost:3000" />
|
||||
</div>
|
||||
<Button onClick={async () => await showHeatmap({ id: "localhost", origin: customOrigin })}>
|
||||
<Button onClick={() => runAsynchronouslyWithAlert(showHeatmap({ id: "localhost", origin: customOrigin }))}>
|
||||
Show heatmap
|
||||
<ArrowRight className="h-4 w-4" />
|
||||
</Button>
|
||||
@ -462,7 +474,7 @@ export default function PageClient() {
|
||||
</Typography>
|
||||
</div>
|
||||
</div>
|
||||
<Button onClick={async () => await showHeatmap(origin)}>
|
||||
<Button onClick={() => runAsynchronouslyWithAlert(showHeatmap(origin))}>
|
||||
Show heatmap
|
||||
<ArrowRight className="h-4 w-4" />
|
||||
</Button>
|
||||
|
||||
@ -2827,6 +2827,13 @@ export const devToolCSS = `
|
||||
color: white;
|
||||
}
|
||||
|
||||
.stack-devtool .sdt-hm-btn-primary:hover {
|
||||
background: var(--sdt-accent);
|
||||
border-color: var(--sdt-accent);
|
||||
color: white;
|
||||
transition: none;
|
||||
}
|
||||
|
||||
.stack-devtool .sdt-hm-stats {
|
||||
flex: 1;
|
||||
min-width: 0;
|
||||
|
||||
@ -292,9 +292,14 @@ export class EventTracker {
|
||||
|
||||
while (current && depth < 8 && current !== document.documentElement) {
|
||||
let part = current.tagName.toLowerCase();
|
||||
const testId = current.getAttribute("data-testid") ?? current.getAttribute("data-test-id");
|
||||
let testIdAttr = "data-testid";
|
||||
let testId = current.getAttribute("data-testid");
|
||||
if (testId == null) {
|
||||
testIdAttr = "data-test-id";
|
||||
testId = current.getAttribute("data-test-id");
|
||||
}
|
||||
if (testId != null && testId.trim() !== "") {
|
||||
part += `[data-testid="${testId.replace(/"/g, '\\"')}"]`;
|
||||
part += `[${testIdAttr}="${testId.replace(/"/g, '\\"')}"]`;
|
||||
parts.unshift(part);
|
||||
break;
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user