fix(api): don't poison the VIN negative cache on transient source failures

A decode that failed from a transient proxy/network blip (PartsCatalogs
DataImpulse ConnectTimeout, EMEX timeout, PL24 transport error) was written to
the 6h negative cache identically to a genuine "brand not supported" miss. A
user who hit a blip then got "Şase tanınamadı" for 6h with no way to retry out —
and prod analytics showed this hitting SUPPORTED brands (Fiat/Toyota/Hyundai
decode fine once the proxy responds).

Thread a `transient` outcome flag through resolveVin -> doResolveVin:
- PartsCatalogs.decodeVin sets it when it swallows a transport error (vs a clean
  "no cars" miss), via a new optional outcome param.
- EMEX surfaces it via its existing {type:"error"} result.
- PL24 sets it on transport/timeout errors in the fallback catch.
resolveVin then skips the negative cache when transient=true; genuine misses
still cache for 6h. Phase 1 of 4 on decode reliability.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-25 00:08:08 +03:00
parent 6283b66502
commit 257052162e
2 changed files with 46 additions and 7 deletions

View File

@@ -41,7 +41,11 @@ export class PartsCatalogsService {
/**
* VIN decode — returns one or more car matches.
*/
async decodeVin(vin: string, signal?: AbortSignal): Promise<PcatVinResult | null> {
async decodeVin(
vin: string,
signal?: AbortSignal,
outcome?: { transient: boolean },
): Promise<PcatVinResult | null> {
try {
const data = await this.fetchWithAuth("/car/info", { q: vin }, signal);
@@ -75,7 +79,21 @@ export class PartsCatalogsService {
return { cars };
} catch (err) {
this.logger.warn(`VIN decode failed for ${vin}: ${(err as Error).message}`);
const e = err as Error & { cause?: unknown };
// Distinguish a transient transport blip (proxy timeout / dropped DataImpulse
// connection) from a genuine "not found". The caller uses `outcome.transient`
// to decide whether to poison the negative cache — a blip must not.
if (
outcome &&
(e.name === "TimeoutError" ||
e.name === "TypeError" ||
/fetch failed|ECONNRESET|ECONNREFUSED|EAI_AGAIN|socket hang up|other side closed|terminated|UND_ERR|ConnectTimeout|aborted/i.test(
`${e.message} ${String(e.cause ?? "")}`,
))
) {
outcome.transient = true;
}
this.logger.warn(`VIN decode failed for ${vin}: ${e.message}`);
return null;
}
}

View File

@@ -414,8 +414,12 @@ export class VehiclesService {
const budgetTimer = setTimeout(() => ac.abort(), VehiclesService.RESOLVE_BUDGET_MS);
if (ctx && !ctx.timings.cache_source) ctx.timings.cache_source = "miss";
// Tracks whether any source failed transiently (proxy/network/timeout) during
// this resolve. If so, a null result is NOT a definitive "unsupported" answer
// and must not be written to the negative cache (which would persist the blip).
const outcome = { transient: false };
try {
const result = await this.doResolveVin(vin, userId, ctx, ac.signal);
const result = await this.doResolveVin(vin, userId, ctx, ac.signal, outcome);
if (ac.signal.aborted) {
this.logger.warn(
`VIN ${vin} decode aborted by budget (${VehiclesService.RESOLVE_BUDGET_MS}ms)`,
@@ -438,6 +442,11 @@ export class VehiclesService {
}
if (result) {
await this.redis.setJson(cacheKey, result, VehiclesService.RESOLVE_TTL_POSITIVE_S);
} else if (outcome.transient) {
// A source failed transiently (proxy/network/timeout); the VIN may well be
// supported. Don't poison the negative cache — let the next attempt retry.
this.logger.warn(`VIN ${vin}: skipping negative cache — transient source failure`);
if (ctx) ctx.timings.neg_cache_skipped = 1;
} else {
await this.redis.set(negKey, "1", VehiclesService.RESOLVE_TTL_NEGATIVE_S);
}
@@ -497,6 +506,7 @@ export class VehiclesService {
userId?: string,
ctx?: ResolveContext,
signal?: AbortSignal,
outcome?: { transient: boolean },
): Promise<VinResolveResult | null> {
const corgiKnown = false;
let brandName: string | null = null;
@@ -507,7 +517,7 @@ export class VehiclesService {
const pcatStart = Date.now();
let pcatResolved = false;
const pcatPromise = this.partsCatalogsService
.decodeVin(vin, signal)
.decodeVin(vin, signal, outcome)
.catch((err: Error) => {
this.logger.warn(`PartsCatalogs decode failed for ${vin}: ${err.message}`);
return null;
@@ -621,8 +631,7 @@ export class VehiclesService {
// engineCode is alphanumeric (e.g. "N20B20") — leave raw; only the
// engineType fallback ("Petrol"/"Diesel") goes through the dict.
engine:
emexSingleVehicle.engineCode ||
translateEngineType(emexSingleVehicle.engineType || null),
emexSingleVehicle.engineCode || translateEngineType(emexSingleVehicle.engineType || null),
transmission: translateTransmission(emexSingleVehicle.transmission || null),
bodyType: translateBodyType(emexSingleVehicle.bodyType || null),
rawData: emexSingleVehicle.raw || null,
@@ -635,6 +644,8 @@ export class VehiclesService {
// Decision 3: EMEX timed out / returned multiple / returned nothing → PL24
this.logger.log(`PL24 fallback triggered for ${vin} (EMEX: ${emexResult?.type ?? "timeout"})`);
// EMEX caught an internal failure (timeout/transport) → not a definitive miss.
if (outcome && emexResult?.type === "error") outcome.transient = true;
const pl24CircuitOpen = await this.isPl24CircuitOpen();
if (ctx) ctx.timings.pl24_circuit_open = pl24CircuitOpen;
@@ -680,7 +691,17 @@ export class VehiclesService {
} catch (err) {
if (ctx) ctx.timings.pl24 = Date.now() - pl24Start;
await this.recordPl24Failure();
this.logger.warn(`PL24 decode failed for ${vin}: ${(err as Error).message}`);
const e = err as Error & { cause?: unknown };
if (
outcome &&
(e.name === "TimeoutError" ||
/fetch failed|ECONNRESET|ECONNREFUSED|EAI_AGAIN|terminated|UND_ERR|ConnectTimeout|timeout|aborted/i.test(
`${e.message} ${String(e.cause ?? "")}`,
))
) {
outcome.transient = true;
}
this.logger.warn(`PL24 decode failed for ${vin}: ${e.message}`);
}
}