From 39eb037b8b030831f48d085af19629a63eceab7d Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Wed, 8 Oct 2025 16:55:44 -0600 Subject: [PATCH] simple cache tag to dedupe --- apps/server/src/api/v2/dashboards/[id]/GET.ts | 2 +- .../src/api/v2/metric_files/[id]/data/GET.ts | 2 +- apps/server/src/api/v2/reports/[id]/GET.ts | 2 +- apps/server/src/shared-helpers/screenshots.ts | 34 +++++++++++++------ .../screenshots/reports.$reportId.content.tsx | 20 +++++++---- 5 files changed, 40 insertions(+), 20 deletions(-) diff --git a/apps/server/src/api/v2/dashboards/[id]/GET.ts b/apps/server/src/api/v2/dashboards/[id]/GET.ts index d91a6cfb1..db3d89982 100644 --- a/apps/server/src/api/v2/dashboards/[id]/GET.ts +++ b/apps/server/src/api/v2/dashboards/[id]/GET.ts @@ -247,7 +247,7 @@ export async function getDashboardHandler( workspace_member_count: workspaceMemberCount, }; - const tag = `take-dashboard-screenshot-${dashboardId}-${versionNumber}`; + const tag = `take-dashboard-screenshot-${dashboardId}`; if ( await shouldTakeScreenshot({ tag, diff --git a/apps/server/src/api/v2/metric_files/[id]/data/GET.ts b/apps/server/src/api/v2/metric_files/[id]/data/GET.ts index 84a8355b1..4cd6d14f7 100644 --- a/apps/server/src/api/v2/metric_files/[id]/data/GET.ts +++ b/apps/server/src/api/v2/metric_files/[id]/data/GET.ts @@ -29,7 +29,7 @@ const app = new Hono() password ); - const tag = `take-metric-screenshot-${id}-${version_number}`; + const tag = `take-metric-screenshot-${id}`; if ( await shouldTakeScreenshot({ tag, diff --git a/apps/server/src/api/v2/reports/[id]/GET.ts b/apps/server/src/api/v2/reports/[id]/GET.ts index d53ca4911..e6fd05936 100644 --- a/apps/server/src/api/v2/reports/[id]/GET.ts +++ b/apps/server/src/api/v2/reports/[id]/GET.ts @@ -91,7 +91,7 @@ const app = new Hono() password ); - const tag = `take-report-screenshot-${reportId}-${versionNumber}`; + const tag = `take-report-screenshot-${reportId}`; if ( await shouldTakeScreenshot({ diff --git a/apps/server/src/shared-helpers/screenshots.ts b/apps/server/src/shared-helpers/screenshots.ts index 44c3dca14..884b7a0c6 100644 --- a/apps/server/src/shared-helpers/screenshots.ts +++ b/apps/server/src/shared-helpers/screenshots.ts @@ -5,6 +5,9 @@ import type { Context } from 'hono'; // This helper ensures that we do not run multiple trigger jobs for the same screenshot task concurrently. // It checks if a job for the given tag and key is already running or queued before starting a new one. +const currentlyCheckingTags = new Set(); +const CACHE_TAG_EXPIRATION_TIME = 1000 * 15; // 15 seconds + export const shouldTakeScreenshot = async ({ tag, key, @@ -16,18 +19,29 @@ export const shouldTakeScreenshot = async ({ }): Promise => { const hasReferrer = !!context.req.header('referer'); - if (!hasReferrer) { + if (!hasReferrer || currentlyCheckingTags.has(tag)) { return false; } - const lastTask = await runs - .list({ - status: ['EXECUTING', 'QUEUED'], - taskIdentifier: key, - tag, - limit: 1, - }) - .then((res) => res.data[0]); + currentlyCheckingTags.add(tag); - return !lastTask; + try { + const lastTask = await runs + .list({ + status: ['EXECUTING', 'QUEUED'], + taskIdentifier: key, + tag, + limit: 1, + }) + .then((res) => res.data[0]); + + return !lastTask; + } catch (error) { + console.error('Error checking if screenshot should be taken', { error }); + throw error; + } finally { + setTimeout(() => { + currentlyCheckingTags.delete(tag); + }, CACHE_TAG_EXPIRATION_TIME); + } }; diff --git a/apps/web/src/routes/screenshots/reports.$reportId.content.tsx b/apps/web/src/routes/screenshots/reports.$reportId.content.tsx index 66cf09be6..642a32171 100644 --- a/apps/web/src/routes/screenshots/reports.$reportId.content.tsx +++ b/apps/web/src/routes/screenshots/reports.$reportId.content.tsx @@ -8,12 +8,14 @@ export const Route = createFileRoute('/screenshots/reports/$reportId/content')({ component: RouteComponent, validateSearch: GetReportScreenshotQuerySchema, ssr: true, - beforeLoad: async ({ context, params, search }) => { - const report = await prefetchGetReport( - context.queryClient, - params.reportId, - search.version_number - ); + beforeLoad: ({ search }) => { + return { + version_number: search.version_number, + }; + }, + loader: async ({ context, params }) => { + const { version_number } = context; + const report = await prefetchGetReport(context.queryClient, params.reportId, version_number); if (!report) { throw redirect({ @@ -23,7 +25,7 @@ export const Route = createFileRoute('/screenshots/reports/$reportId/content')({ const allMetrics = Object.entries(report?.metrics || {}); - await Promise.all( + const res = await Promise.all( allMetrics.map(([metricId, metric]) => { return ensureMetricData(context.queryClient, { id: metricId, @@ -32,6 +34,10 @@ export const Route = createFileRoute('/screenshots/reports/$reportId/content')({ }) ); + res.forEach((metric) => { + console.log('metric', metric.data?.length); + }); + return { report }; }, });