DataForSEO's own server errors show as an unexpected error and retry 3x (#526)
This commit is contained in:
parent
67d68281e3
commit
94b730124b
@ -22,6 +22,9 @@ export function useKeywordSerpAnalysis(
|
|||||||
},
|
},
|
||||||
}),
|
}),
|
||||||
enabled: !!serpKeyword,
|
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 ?? [];
|
const serpResults = serpQuery.data?.items ?? [];
|
||||||
|
|||||||
@ -114,7 +114,7 @@ describe("subscription billing", () => {
|
|||||||
|
|
||||||
const result = assertUsageCreditsAvailable("org_123");
|
const result = assertUsageCreditsAvailable("org_123");
|
||||||
const assertion = expect(result).rejects.toMatchObject({
|
const assertion = expect(result).rejects.toMatchObject({
|
||||||
code: "UPSTREAM_UNAVAILABLE",
|
code: "INTERNAL_ERROR",
|
||||||
});
|
});
|
||||||
await vi.runAllTimersAsync();
|
await vi.runAllTimersAsync();
|
||||||
|
|
||||||
|
|||||||
@ -130,8 +130,9 @@ async function getUsageCreditsRemaining(customerId: string): Promise<{
|
|||||||
// credits out of chat (2026-07-20). The topup balance genuinely doesn't
|
// 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.
|
// exist until a first top-up, so 0 is the honest reading there.
|
||||||
if (!monthlyBalance) {
|
if (!monthlyBalance) {
|
||||||
|
// INTERNAL_ERROR, not UPSTREAM_UNAVAILABLE: this must stay reportable.
|
||||||
throw new AppError(
|
throw new AppError(
|
||||||
"UPSTREAM_UNAVAILABLE",
|
"INTERNAL_ERROR",
|
||||||
`Autumn check returned no ${AUTUMN_SEO_DATA_BALANCE_FEATURE_ID} balance for customer ${customerId}`,
|
`Autumn check returned no ${AUTUMN_SEO_DATA_BALANCE_FEATURE_ID} balance for customer ${customerId}`,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@ -110,6 +110,14 @@ function createAuthenticatedFetch(
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
error.name = "DataForSEOHttpError";
|
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;
|
throw error;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
@ -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", () => {
|
it("appends the echoed request value to opaque 'Invalid Field' failures", () => {
|
||||||
const task = {
|
const task = {
|
||||||
status_code: 40501,
|
status_code: 40501,
|
||||||
|
|||||||
@ -1,6 +1,7 @@
|
|||||||
import { z } from "zod";
|
import { z } from "zod";
|
||||||
import { AppError } from "@/server/lib/errors";
|
import { AppError } from "@/server/lib/errors";
|
||||||
import type { DataforseoErrorClassifier } from "@/server/lib/dataforseo/core";
|
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
|
// 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.
|
* a non-reportable VALIDATION_ERROR.
|
||||||
*/
|
*/
|
||||||
public readonly isInvalidField = false,
|
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";
|
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 lifecycle codes meaning "not done yet": Task Created / Task Handed /
|
||||||
* Task In Queue. A task_get returning one of these is pending, not failed. */
|
* 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]);
|
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
|
* returns that (SDK-typed) task. The single status / billing ladder shared by
|
||||||
* every endpoint:
|
* every endpoint:
|
||||||
* - access / balance failure -> classified AppError
|
* - access / balance failure -> classified AppError
|
||||||
|
* - DataForSEO's own backend erring (5xxxx) -> UPSTREAM_UNAVAILABLE
|
||||||
* - charged-but-failed task (cost present) -> DataforseoChargedTaskError
|
* - charged-but-failed task (cost present) -> DataforseoChargedTaskError
|
||||||
*/
|
*/
|
||||||
export function assertOk<T extends DataforseoTaskLike>(
|
export function assertOk<T extends DataforseoTaskLike>(
|
||||||
@ -203,15 +244,30 @@ export function assertOk<T extends DataforseoTaskLike>(
|
|||||||
if (classified) throw classified;
|
if (classified) throw classified;
|
||||||
|
|
||||||
const detailedMessage = describeInvalidField(message, task);
|
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);
|
const billing = tryBuildTaskBilling(task);
|
||||||
if (billing)
|
if (billing)
|
||||||
throw new DataforseoChargedTaskError(
|
throw new DataforseoChargedTaskError(
|
||||||
detailedMessage,
|
detailedMessage,
|
||||||
billing,
|
billing,
|
||||||
INVALID_FIELD_MESSAGE_RE.test(message),
|
INVALID_FIELD_MESSAGE_RE.test(message),
|
||||||
|
code,
|
||||||
);
|
);
|
||||||
|
|
||||||
throw new AppError("INTERNAL_ERROR", detailedMessage);
|
throw new AppError(code, detailedMessage);
|
||||||
}
|
}
|
||||||
|
|
||||||
return task;
|
return task;
|
||||||
|
|||||||
@ -11,6 +11,7 @@ describe("shouldCaptureAppErrorCode", () => {
|
|||||||
"AUDIT_PAGE_LIMIT_EXCEEDED",
|
"AUDIT_PAGE_LIMIT_EXCEEDED",
|
||||||
"AUDIT_ALREADY_RUNNING",
|
"AUDIT_ALREADY_RUNNING",
|
||||||
"RATE_LIMITED",
|
"RATE_LIMITED",
|
||||||
|
"UPSTREAM_UNAVAILABLE",
|
||||||
] as const)("skips expected %s errors", (code) => {
|
] as const)("skips expected %s errors", (code) => {
|
||||||
expect(shouldCaptureAppErrorCode(code)).toBe(false);
|
expect(shouldCaptureAppErrorCode(code)).toBe(false);
|
||||||
});
|
});
|
||||||
|
|||||||
@ -37,6 +37,11 @@ const NON_REPORTABLE_ERROR_CODES = new Set<ErrorCode>([
|
|||||||
"AUDIT_CAPACITY_REACHED",
|
"AUDIT_CAPACITY_REACHED",
|
||||||
"AUDIT_PAGE_LIMIT_EXCEEDED",
|
"AUDIT_PAGE_LIMIT_EXCEEDED",
|
||||||
"AUDIT_ALREADY_RUNNING",
|
"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 {
|
export function isErrorCode(value: string): value is ErrorCode {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user