From 94b730124b7186d2dd437984453d0546baee158d Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:06:40 -0400 Subject: [PATCH] DataForSEO's own server errors show as an unexpected error and retry 3x (#526) --- .../keywords/hooks/useKeywordSerpAnalysis.ts | 3 + src/server/billing/subscription.test.ts | 2 +- src/server/billing/subscription.ts | 3 +- src/server/lib/dataforseo/core.ts | 8 +++ src/server/lib/dataforseo/envelope.test.ts | 40 +++++++++++++ src/server/lib/dataforseo/envelope.ts | 60 ++++++++++++++++++- src/shared/error-codes.test.ts | 1 + src/shared/error-codes.ts | 5 ++ 8 files changed, 118 insertions(+), 4 deletions(-) diff --git a/src/client/features/keywords/hooks/useKeywordSerpAnalysis.ts b/src/client/features/keywords/hooks/useKeywordSerpAnalysis.ts index 6f01ce1..12dd8a6 100644 --- a/src/client/features/keywords/hooks/useKeywordSerpAnalysis.ts +++ b/src/client/features/keywords/hooks/useKeywordSerpAnalysis.ts @@ -22,6 +22,9 @@ export function useKeywordSerpAnalysis( }, }), enabled: !!serpKeyword, + // Every attempt is a fresh billed DataForSEO task, so a failure must not be + // retried automatically — the user re-runs the search instead. + retry: false, }); const serpResults = serpQuery.data?.items ?? []; diff --git a/src/server/billing/subscription.test.ts b/src/server/billing/subscription.test.ts index baf2291..c25c059 100644 --- a/src/server/billing/subscription.test.ts +++ b/src/server/billing/subscription.test.ts @@ -114,7 +114,7 @@ describe("subscription billing", () => { const result = assertUsageCreditsAvailable("org_123"); const assertion = expect(result).rejects.toMatchObject({ - code: "UPSTREAM_UNAVAILABLE", + code: "INTERNAL_ERROR", }); await vi.runAllTimersAsync(); diff --git a/src/server/billing/subscription.ts b/src/server/billing/subscription.ts index a555254..bca3099 100644 --- a/src/server/billing/subscription.ts +++ b/src/server/billing/subscription.ts @@ -130,8 +130,9 @@ async function getUsageCreditsRemaining(customerId: string): Promise<{ // credits out of chat (2026-07-20). The topup balance genuinely doesn't // exist until a first top-up, so 0 is the honest reading there. if (!monthlyBalance) { + // INTERNAL_ERROR, not UPSTREAM_UNAVAILABLE: this must stay reportable. throw new AppError( - "UPSTREAM_UNAVAILABLE", + "INTERNAL_ERROR", `Autumn check returned no ${AUTUMN_SEO_DATA_BALANCE_FEATURE_ID} balance for customer ${customerId}`, ); } diff --git a/src/server/lib/dataforseo/core.ts b/src/server/lib/dataforseo/core.ts index 63061d9..87d68ce 100644 --- a/src/server/lib/dataforseo/core.ts +++ b/src/server/lib/dataforseo/core.ts @@ -110,6 +110,14 @@ function createAuthenticatedFetch( }, ); error.name = "DataForSEOHttpError"; + // UPSTREAM_UNAVAILABLE is non-reportable, and the error handlers only log + // what they capture, so log here to keep the provider's failure rate + // visible in Workers Observability. + if (code === "UPSTREAM_UNAVAILABLE") + console.error("dataforseo.upstream-http-failed", { + path, + status: response.status, + }); throw error; } }; diff --git a/src/server/lib/dataforseo/envelope.test.ts b/src/server/lib/dataforseo/envelope.test.ts index d9b2f68..e4f8deb 100644 --- a/src/server/lib/dataforseo/envelope.test.ts +++ b/src/server/lib/dataforseo/envelope.test.ts @@ -60,6 +60,46 @@ describe("assertOk", () => { } }); + it("classifies DataForSEO's own server errors as UPSTREAM_UNAVAILABLE", () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + const task = { + status_code: 40101, + status_message: "Internal SE Server Error.", + path: ["v3", "serp", "google", "organic", "live", "advanced"], + cost: 0.002, + result_count: 0, + }; + try { + assertOk({ status_code: 20000, tasks: [task] }); + throw new Error("expected assertOk to throw"); + } catch (error) { + // Still a charged-task error so the billed attempt stays metered. + expect(error).toBeInstanceOf(DataforseoChargedTaskError); + if (error instanceof DataforseoChargedTaskError) { + expect(error.code).toBe("UPSTREAM_UNAVAILABLE"); + } + } + }); + + it("keeps 'Not Implemented' reportable — we posted a bad task", () => { + const task = { + status_code: 50100, + status_message: "Not Implemented.", + path: ["v3", "serp", "google", "organic", "live", "advanced"], + cost: 0.002, + result_count: 0, + }; + try { + assertOk({ status_code: 20000, tasks: [task] }); + throw new Error("expected assertOk to throw"); + } catch (error) { + expect(error).toBeInstanceOf(DataforseoChargedTaskError); + if (error instanceof DataforseoChargedTaskError) { + expect(error.code).toBe("INTERNAL_ERROR"); + } + } + }); + it("appends the echoed request value to opaque 'Invalid Field' failures", () => { const task = { status_code: 40501, diff --git a/src/server/lib/dataforseo/envelope.ts b/src/server/lib/dataforseo/envelope.ts index e1ddf93..a3a8a25 100644 --- a/src/server/lib/dataforseo/envelope.ts +++ b/src/server/lib/dataforseo/envelope.ts @@ -1,6 +1,7 @@ import { z } from "zod"; import { AppError } from "@/server/lib/errors"; import type { DataforseoErrorClassifier } from "@/server/lib/dataforseo/core"; +import type { ErrorCode } from "@/shared/error-codes"; // --------------------------------------------------------------------------- // Billing envelope — the load-bearing seam that carries each call's USD cost @@ -36,8 +37,15 @@ export class DataforseoChargedTaskError extends AppError { * a non-reportable VALIDATION_ERROR. */ public readonly isInvalidField = false, + /** + * Classification for the failure. Defaults to INTERNAL_ERROR (our bug); + * DataForSEO's own backend failures pass UPSTREAM_UNAVAILABLE so the user + * sees "provider temporarily unavailable" and the flake isn't captured as + * an app exception. + */ + code: ErrorCode = "INTERNAL_ERROR", ) { - super("INTERNAL_ERROR", message); + super(code, message); this.name = "DataforseoChargedTaskError"; } } @@ -138,6 +146,38 @@ export function isNoResultsTask(task: DataforseoTaskLike): boolean { ); } +/** + * Status codes where DataForSEO's own backend failed, returned on an HTTP 200 + * with a failed task. These are provider flakes, not our bug, so they classify + * as UPSTREAM_UNAVAILABLE: the customer gets the retry-in-a-moment message + * instead of a generic "unexpected error", and the flake isn't captured. + * + * An explicit list, not a `>= 50000` range: 40101 "Internal SE Server Error." + * is the one that actually fires (by far our loudest captured exception) and it + * sits in the 40000 family, while 50100 "Not Implemented." means we posted a + * non-existing task or parameter — our bug, and it must stay reportable. + * Likewise 50001 "Error While Checking the Balance." stays visible. + * @see https://docs.dataforseo.com/v3/appendix/errors/ + */ +const UPSTREAM_FAILURE_STATUS_CODES = new Set([ + 40101, // Internal SE Server Error. + 40103, // Task execution failed, please try to resubmit. + 50000, // Internal Error. + 50301, // 3rd Party API Service Unavailable. + 50302, // Internal 3rd Party API Service Unavailable. + 50303, // Update in progress. Please try after a few minutes. + 50304, // This function temporarily unavailable. + 50401, // Internal Error - Timeout. + 50402, // Target page took too long to respond. +]); + +function isUpstreamServerErrorTask(task: DataforseoTaskLike): boolean { + return ( + task.status_code !== undefined && + UPSTREAM_FAILURE_STATUS_CODES.has(task.status_code) + ); +} + /** Task lifecycle codes meaning "not done yet": Task Created / Task Handed / * Task In Queue. A task_get returning one of these is pending, not failed. */ const TASK_IN_PROGRESS_STATUS_CODES = new Set([20100, 40601, 40602]); @@ -166,6 +206,7 @@ type AssertOkOptions = { * returns that (SDK-typed) task. The single status / billing ladder shared by * every endpoint: * - access / balance failure -> classified AppError + * - DataForSEO's own backend erring (5xxxx) -> UPSTREAM_UNAVAILABLE * - charged-but-failed task (cost present) -> DataforseoChargedTaskError */ export function assertOk( @@ -203,15 +244,30 @@ export function assertOk( if (classified) throw classified; const detailedMessage = describeInvalidField(message, task); + const isUpstreamFailure = isUpstreamServerErrorTask(task); + // UPSTREAM_UNAVAILABLE is non-reportable, and the error handlers only log + // what they capture, so log here to keep the provider's failure rate — and + // the only remaining record of the message — visible in Workers Observability. + if (isUpstreamFailure) + console.error("dataforseo.upstream-task-failed", { + path, + status: task.status_code, + message: task.status_message, + }); + const code: ErrorCode = isUpstreamFailure + ? "UPSTREAM_UNAVAILABLE" + : "INTERNAL_ERROR"; + const billing = tryBuildTaskBilling(task); if (billing) throw new DataforseoChargedTaskError( detailedMessage, billing, INVALID_FIELD_MESSAGE_RE.test(message), + code, ); - throw new AppError("INTERNAL_ERROR", detailedMessage); + throw new AppError(code, detailedMessage); } return task; diff --git a/src/shared/error-codes.test.ts b/src/shared/error-codes.test.ts index ee5b850..085f275 100644 --- a/src/shared/error-codes.test.ts +++ b/src/shared/error-codes.test.ts @@ -11,6 +11,7 @@ describe("shouldCaptureAppErrorCode", () => { "AUDIT_PAGE_LIMIT_EXCEEDED", "AUDIT_ALREADY_RUNNING", "RATE_LIMITED", + "UPSTREAM_UNAVAILABLE", ] as const)("skips expected %s errors", (code) => { expect(shouldCaptureAppErrorCode(code)).toBe(false); }); diff --git a/src/shared/error-codes.ts b/src/shared/error-codes.ts index c4c34bb..dd95158 100644 --- a/src/shared/error-codes.ts +++ b/src/shared/error-codes.ts @@ -37,6 +37,11 @@ const NON_REPORTABLE_ERROR_CODES = new Set([ "AUDIT_CAPACITY_REACHED", "AUDIT_PAGE_LIMIT_EXCEEDED", "AUDIT_ALREADY_RUNNING", + // An external provider (DataForSEO) failing on its own side. Nothing in the + // app to fix, and it drowned real exceptions. Note the error handlers only + // log what they capture, so every throw site logs its own line to keep the + // failure rate visible in Workers Observability. + "UPSTREAM_UNAVAILABLE", ]); export function isErrorCode(value: string): value is ErrorCode {