From c9bcc444b48447260b31c21544faf7c95531c287 Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Sun, 5 Jul 2026 22:38:53 -0400 Subject: [PATCH] Triage unresolved PostHog errors: DataForSEO validation, DO retry, Autumn 429, exception noise (#333) --- src/client/lib/posthog.ts | 1 + .../onboarding/OnboardingChatAgent.ts | 22 +++++++ src/server/lib/dataforseo/client.test.ts | 45 ++++++++++++++ src/server/lib/dataforseo/client.ts | 9 +++ src/server/lib/dataforseo/envelope.ts | 18 +++++- src/server/mcp/schemas.ts | 27 +++++++++ .../mcp/tools/dataforseo-research-tools.ts | 2 + .../tools/get-domain-keyword-suggestions.ts | 2 + src/server/mcp/tools/get-domain-overview.ts | 2 + src/server/mcp/tools/research-keywords.ts | 2 + src/serverFunctions/billing.ts | 59 ++++++++++++------- 11 files changed, 167 insertions(+), 22 deletions(-) diff --git a/src/client/lib/posthog.ts b/src/client/lib/posthog.ts index d569062..077758d 100644 --- a/src/client/lib/posthog.ts +++ b/src/client/lib/posthog.ts @@ -30,6 +30,7 @@ function isIgnorableException( const value = typeof entry?.value === "string" ? entry.value : ""; if (value.includes("Object Not Found Matching Id")) return true; if (value === "Script error.") return true; + if (value.includes("signal is aborted without reason")) return true; const frames = entry?.stacktrace?.frames; return ( value === "undefined" && diff --git a/src/server/features/onboarding/OnboardingChatAgent.ts b/src/server/features/onboarding/OnboardingChatAgent.ts index 64100e0..89a8b67 100644 --- a/src/server/features/onboarding/OnboardingChatAgent.ts +++ b/src/server/features/onboarding/OnboardingChatAgent.ts @@ -71,6 +71,28 @@ export class OnboardingChatAgent extends AIChatAgent { // Cap stored history; the onboarding chat is short and pre-paywall. maxPersistedMessages = 60; + // The base class persists each message as its own bounded SQLite row, so DO + // storage occasionally returns a transient internal error (code 10001) that + // clears on retry. Retry the message-write path a couple of times before + // surfacing the failure, rethrowing on non-transient errors or the last try. + async persistMessages( + ...args: Parameters + ): Promise { + const maxAttempts = 3; + for (let attempt = 1; ; attempt++) { + try { + await super.persistMessages(...args); + return; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + const transient = + message.includes("internal error") || message.includes("10001"); + if (!transient || attempt >= maxAttempts) throw error; + await new Promise((resolve) => setTimeout(resolve, 50 * attempt)); + } + } + } + async onChatMessage( onFinish: StreamTextOnFinishCallback, options?: OnChatMessageOptions, diff --git a/src/server/lib/dataforseo/client.test.ts b/src/server/lib/dataforseo/client.test.ts index 6342738..40553bf 100644 --- a/src/server/lib/dataforseo/client.test.ts +++ b/src/server/lib/dataforseo/client.test.ts @@ -274,6 +274,51 @@ describe("meterDataforseoCall with split balances", () => { ); }); + it("skips the charge for an unbilled invalid-field failure and rethrows VALIDATION_ERROR", async () => { + setupHostedMode(); + mockBalances(5000, 3000); + vi.mocked(fetchBacklinksSummary).mockRejectedValue( + new DataforseoChargedTaskError( + "Invalid Field: 'target'.", + { costUsd: 0, path: ["v3", "backlinks", "summary", "live"] }, + true, + ), + ); + + const client = createDataforseoClient(billingCustomer); + await expect( + client.backlinks.summary(backlinksInput), + ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); + + expect(trackMock).not.toHaveBeenCalled(); + }); + + it("still meters an invalid-field failure that DataForSEO actually billed", async () => { + setupHostedMode(); + mockBalances(5000, 3000); + vi.mocked(fetchBacklinksSummary).mockRejectedValue( + new DataforseoChargedTaskError( + "Invalid Field: 'target'.", + { costUsd: RAW_COST, path: ["v3", "backlinks", "summary", "live"] }, + true, + ), + ); + + const client = createDataforseoClient(billingCustomer); + await expect(client.backlinks.summary(backlinksInput)).rejects.toThrow( + "Invalid Field: 'target'.", + ); + + expect(trackMock).toHaveBeenCalledTimes(1); + expect(trackMock).toHaveBeenCalledWith( + expect.objectContaining({ + customerId: "org_123", + featureId: AUTUMN_SEO_DATA_BALANCE_FEATURE_ID, + value: EXPECTED_CREDITS, + }), + ); + }); + it("includes balanceFeatureId in track properties", async () => { setupHostedMode(); mockBalances(30, 5000); diff --git a/src/server/lib/dataforseo/client.ts b/src/server/lib/dataforseo/client.ts index bdef76c..8b272fe 100644 --- a/src/server/lib/dataforseo/client.ts +++ b/src/server/lib/dataforseo/client.ts @@ -53,6 +53,7 @@ import { type DataforseoApiResponse, } from "@/server/lib/dataforseo/envelope"; import { isHostedServerAuthMode } from "@/server/lib/runtime-env"; +import { AppError } from "@/server/lib/errors"; export { mapDataforseoPathToCreditFeature }; @@ -161,6 +162,14 @@ async function meterDataforseoCall( result = await execute(); } catch (error) { if (error instanceof DataforseoChargedTaskError) { + // A malformed request (DataForSEO "Invalid Field: ...") that DataForSEO + // did not bill returns no value to the customer, so don't charge — surface + // it as a non-reportable VALIDATION_ERROR. If DataForSEO still billed us + // (costUsd > 0), fall through to the normal charge + capture path so the + // spend stays metered and visible instead of silently eaten. + if (error.isInvalidField && error.billing.costUsd <= 0) { + throw new AppError("VALIDATION_ERROR", error.message); + } await trackDataforseoCost({ customer, customerId: billingCustomer.id, diff --git a/src/server/lib/dataforseo/envelope.ts b/src/server/lib/dataforseo/envelope.ts index ae61344..c698ad8 100644 --- a/src/server/lib/dataforseo/envelope.ts +++ b/src/server/lib/dataforseo/envelope.ts @@ -29,6 +29,13 @@ export class DataforseoChargedTaskError extends AppError { constructor( message: string, public readonly billing: DataforseoApiCallCost, + /** + * True when the task failed because OUR request was malformed (DataForSEO + * "Invalid Field: ..."). The customer got no value, so — when the task + * wasn't billed — meterDataforseoCall skips the charge and rethrows this as + * a non-reportable VALIDATION_ERROR. + */ + public readonly isInvalidField = false, ) { super("INTERNAL_ERROR", message); this.name = "DataforseoChargedTaskError"; @@ -82,6 +89,8 @@ export function buildTaskBilling( return billing; } +const INVALID_FIELD_MESSAGE_RE = /Invalid Field:\s*'([^']+)'/i; + /** * DataForSEO echoes the posted request params back on `task.data`. Its * validation rejections are opaque ("Invalid Field: 'target'.") and name the @@ -93,7 +102,7 @@ function describeInvalidField( message: string, task: DataforseoTaskLike, ): string { - const match = message.match(/Invalid Field:\s*'([^']+)'/i); + const match = message.match(INVALID_FIELD_MESSAGE_RE); if (!match) return message; const field = match[1]; if (!isRecord(task.data)) return message; @@ -165,7 +174,12 @@ export function assertOk( const detailedMessage = describeInvalidField(message, task); const billing = tryBuildTaskBilling(task); - if (billing) throw new DataforseoChargedTaskError(detailedMessage, billing); + if (billing) + throw new DataforseoChargedTaskError( + detailedMessage, + billing, + INVALID_FIELD_MESSAGE_RE.test(message), + ); throw new AppError("INTERNAL_ERROR", detailedMessage); } diff --git a/src/server/mcp/schemas.ts b/src/server/mcp/schemas.ts index a76ee3c..7c95dea 100644 --- a/src/server/mcp/schemas.ts +++ b/src/server/mcp/schemas.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import { AppError } from "@/server/lib/errors"; import { getKeywordDataProvider, + getLanguageOptions, isSupportedLanguageCode, } from "@/shared/keyword-locations"; @@ -36,6 +37,32 @@ export function assertLabsLocationCode(locationCode: number | undefined) { } } +/** + * Guards Labs-backed tools against a language DataForSEO doesn't serve for the + * chosen location. A mismatched pair (e.g. language_code="ru" for the United + * States) is otherwise rejected as an opaque *charged* "Invalid Field: + * 'language_code'." task failure, so validate the pair first (cost 0). Only + * Labs locations have authoritative per-location language lists; Google Ads + * locations are left to the metering safety net. + */ +export function assertLanguageForLocation( + locationCode: number | undefined, + languageCode: string | undefined, +) { + if (languageCode == null) return; + const resolvedLocation = locationCode ?? DEFAULT_LOCATION_CODE; + if (getKeywordDataProvider(resolvedLocation) !== "labs") return; + const options = getLanguageOptions(resolvedLocation); + if (!options.some((option) => option.code === languageCode)) { + throw new AppError( + "VALIDATION_ERROR", + `Language '${languageCode}' is not available for this location. Available: ${options + .map((option) => option.code) + .join(", ")}.`, + ); + } +} + export const languageCodeSchema = z .string() .refine(isSupportedLanguageCode, { diff --git a/src/server/mcp/tools/dataforseo-research-tools.ts b/src/server/mcp/tools/dataforseo-research-tools.ts index 532bfca..7d79626 100644 --- a/src/server/mcp/tools/dataforseo-research-tools.ts +++ b/src/server/mcp/tools/dataforseo-research-tools.ts @@ -20,6 +20,7 @@ import { import { DEFAULT_LANGUAGE_CODE, DEFAULT_LOCATION_CODE, + assertLanguageForLocation, languageCodeSchema, locationCodeSchema, projectIdSchema, @@ -828,6 +829,7 @@ export const getKeywordMetricsTool = { }, }, handler: withMcpProjectAuth(async (args: GetKeywordMetricsArgs, context) => { + assertLanguageForLocation(args.locationCode, args.languageCode); const client = createDataforseoClient(context.billing); const locationCode = args.locationCode ?? DEFAULT_LOCATION_CODE; const languageCode = args.languageCode ?? DEFAULT_LANGUAGE_CODE; diff --git a/src/server/mcp/tools/get-domain-keyword-suggestions.ts b/src/server/mcp/tools/get-domain-keyword-suggestions.ts index 45c04bc..14f1187 100644 --- a/src/server/mcp/tools/get-domain-keyword-suggestions.ts +++ b/src/server/mcp/tools/get-domain-keyword-suggestions.ts @@ -16,6 +16,7 @@ import { DEFAULT_LANGUAGE_CODE, DEFAULT_LOCATION_CODE, assertLabsLocationCode, + assertLanguageForLocation, languageCodeSchema, locationCodeSchema, projectIdSchema, @@ -59,6 +60,7 @@ export const getDomainKeywordSuggestionsTool = { }, handler: withMcpProjectAuth(async (args: Args, context) => { assertLabsLocationCode(args.locationCode); + assertLanguageForLocation(args.locationCode, args.languageCode); const keywords = await DomainService.getSuggestedKeywords( { domain: args.domain, diff --git a/src/server/mcp/tools/get-domain-overview.ts b/src/server/mcp/tools/get-domain-overview.ts index c0b2263..4970562 100644 --- a/src/server/mcp/tools/get-domain-overview.ts +++ b/src/server/mcp/tools/get-domain-overview.ts @@ -8,6 +8,7 @@ import { DEFAULT_LANGUAGE_CODE, DEFAULT_LOCATION_CODE, assertLabsLocationCode, + assertLanguageForLocation, languageCodeSchema, locationCodeSchema, projectIdSchema, @@ -52,6 +53,7 @@ export const getDomainOverviewTool = { }, handler: withMcpProjectAuth(async (args: Args, context) => { assertLabsLocationCode(args.locationCode); + assertLanguageForLocation(args.locationCode, args.languageCode); const result = await DomainService.getOverview( { projectId: args.projectId, diff --git a/src/server/mcp/tools/research-keywords.ts b/src/server/mcp/tools/research-keywords.ts index ba963d3..503e2ed 100644 --- a/src/server/mcp/tools/research-keywords.ts +++ b/src/server/mcp/tools/research-keywords.ts @@ -11,6 +11,7 @@ import { formatMcpTable, type McpTableColumn } from "@/server/mcp/table"; import { DEFAULT_LANGUAGE_CODE, DEFAULT_LOCATION_CODE, + assertLanguageForLocation, languageCodeSchema, locationCodeSchema, projectIdSchema, @@ -107,6 +108,7 @@ export const researchKeywordsTool = { const results = await Promise.all( args.seeds.map(async (item) => { try { + assertLanguageForLocation(item.locationCode, item.languageCode); const data = await KeywordResearchService.research( { projectId: args.projectId, diff --git a/src/serverFunctions/billing.ts b/src/serverFunctions/billing.ts index 49d09de..1b5b212 100644 --- a/src/serverFunctions/billing.ts +++ b/src/serverFunctions/billing.ts @@ -13,6 +13,11 @@ import { requireAuthenticatedContext } from "@/serverFunctions/middleware"; const AUTUMN_EVENTS_LIST_URL = "https://api.useautumn.com/v1/events.list"; const EVENT_PAGE_LIMIT = 1000; +// Autumn rate-limits events.list; back off on 429 before giving up. +const AUTUMN_MAX_RETRIES = 3; +const AUTUMN_RETRY_BACKOFF_MS = 250; +// Cap a server-supplied Retry-After so a bogus value can't stall the Worker. +const AUTUMN_MAX_RETRY_DELAY_MS = 5000; const BILLING_USAGE_FEATURE_IDS = [ AUTUMN_SEO_DATA_BALANCE_FEATURE_ID, AUTUMN_SEO_DATA_TOPUP_BALANCE_FEATURE_ID, @@ -84,28 +89,42 @@ async function fetchAutumnEventsPage(args: { start: number; }): Promise<{ list: BillingUsageEvent[]; hasMore: boolean }> { const secretKey = await getRequiredEnvValue("AUTUMN_SECRET_KEY"); - const response = await fetch(AUTUMN_EVENTS_LIST_URL, { - method: "POST", - headers: { - Accept: "application/json", - Authorization: `Bearer ${secretKey}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ - customer_id: args.customerId, - custom_range: { - end: args.end, - start: args.start, - }, - feature_id: BILLING_USAGE_FEATURE_IDS, - limit: EVENT_PAGE_LIMIT, - offset: args.offset, - }), - }); - if (!response.ok) { + let response: Response; + for (let attempt = 0; ; attempt++) { + response = await fetch(AUTUMN_EVENTS_LIST_URL, { + method: "POST", + headers: { + Accept: "application/json", + Authorization: `Bearer ${secretKey}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ + customer_id: args.customerId, + custom_range: { + end: args.end, + start: args.start, + }, + feature_id: BILLING_USAGE_FEATURE_IDS, + limit: EVENT_PAGE_LIMIT, + offset: args.offset, + }), + }); + + if (response.ok) break; + + if (response.status === 429 && attempt < AUTUMN_MAX_RETRIES) { + const retryAfterHeader = response.headers.get("Retry-After"); + const retryAfter = retryAfterHeader ? Number(retryAfterHeader) : NaN; + const delayMs = Number.isFinite(retryAfter) + ? Math.min(retryAfter * 1000, AUTUMN_MAX_RETRY_DELAY_MS) + : AUTUMN_RETRY_BACKOFF_MS * (attempt + 1); + await new Promise((resolve) => setTimeout(resolve, delayMs)); + continue; + } + throw new AppError( - "INTERNAL_ERROR", + response.status === 429 ? "RATE_LIMITED" : "INTERNAL_ERROR", `Autumn events.list failed with status ${response.status}`, ); }