fix(vin-decode): 4 RCA-confirmed decode-chain bugs
Some checks failed
QA Gate (P0/P1) / Test affected app (pull_request) Has been cancelled

Root-cause analysis live re-decoded all 124 historically-undecoded prod
VINs; 38 already decode now. These 4 fixes target confirmed code bugs
that drop or mask real decodes (see undecoded-vin-rca.md):

Q2 — PL24 circuit breaker now only counts transient transport faults. A
definitive upstream negative (NotFound/BadRequest) no longer trips the
global 30s breaker that was starving PL24 for every subsequent VIN
(the sibling-VIN inconsistency in the report). Live-proven on VR7.

Q3 — previewVin / multi-candidate path no longer returns an empty
success: the pcat/emex candidate branches fill brandName (from catalogId
/ WMI), fixing the 6 "HTTP 200 with null brand+model" cases.

Q1 — EMEX fetch retries transient proxy failures with a FRESH ProxyAgent
per attempt (rotates the DataImpulse port; ~42% blip rate observed),
plus an opt-in direct fallback (EMEX_DIRECT_FALLBACK). HTTP answers are
never retried.

Q4 — VIN resolve cache keys namespaced by DECODE_CHAIN_VERSION and the
negative TTL drops 6h -> 30m, so a decode-chain fix self-heals stale
negatives on deploy instead of masking phantom-undecoded VINs for hours.
The admin cache-buster uses the same key builder.

Tests: 179 passed (+ new Q2/Q3/Q4 specs). typecheck + biome clean.

Deploy note: prod EMEX_PROXY_PORT_START/END are both 823 (single port);
widen to a real range (e.g. 10001-10099) in Coolify so Q1's port
rotation takes full effect.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-03 22:37:11 +03:00
parent bb60a95ec3
commit a3ae90ec29
6 changed files with 256 additions and 61 deletions

View File

@@ -106,6 +106,14 @@ export class EmexService {
private readonly timeout: number;
private readonly debug: boolean;
private readonly proxyAgent: ProxyAgent | null;
private readonly emexProxy: {
host: string;
user: string;
pass: string;
portStart: number;
portEnd: number;
} | null;
private readonly emexDirectFallback: boolean;
constructor(
private configService: ConfigService,
@@ -122,20 +130,24 @@ export class EmexService {
this.debug = this.configService.get<boolean>("EMEX_DEBUG", false);
const useProxy = this.configService.get<string>("EMEX_USE_PROXY", "true") === "true";
this.emexDirectFallback =
this.configService.get<string>("EMEX_DIRECT_FALLBACK", "false") === "true";
if (useProxy) {
const host = this.configService.get<string>("EMEX_PROXY_HOST", "74.81.81.81");
const portStart = this.configService.get<number>("EMEX_PROXY_PORT_START", 10001);
const portEnd = this.configService.get<number>("EMEX_PROXY_PORT_END", 10099);
const user = this.configService.get<string>("EMEX_PROXY_USER", "1726bbe361918676d44e");
const pass = this.configService.get<string>("EMEX_PROXY_PASS", "f11c7b6128cc86c6");
const port = Math.floor(Math.random() * (portEnd - portStart + 1)) + portStart;
this.proxyAgent = new ProxyAgent({
uri: `http://${user}:${pass}@${host}:${port}`,
connect: { timeout: 30000 },
requestTls: { timeout: 30000 },
});
this.logger.log(`EMEX HTTP proxy enabled: ${host}:${port}`);
this.emexProxy = {
host: this.configService.get<string>("EMEX_PROXY_HOST", "74.81.81.81"),
user: this.configService.get<string>("EMEX_PROXY_USER", "1726bbe361918676d44e"),
pass: this.configService.get<string>("EMEX_PROXY_PASS", "f11c7b6128cc86c6"),
portStart: this.configService.get<number>("EMEX_PROXY_PORT_START", 10001),
portEnd: this.configService.get<number>("EMEX_PROXY_PORT_END", 10099),
};
// Default agent for the low-stakes image-dims path; fetchEmexHtml builds a
// fresh agent per request so a flaky port can't pin every call.
this.proxyAgent = this.newProxyAgent();
this.logger.log(
`EMEX HTTP proxy enabled: ${this.emexProxy.host}:${this.emexProxy.portStart}-${this.emexProxy.portEnd}`,
);
} else {
this.emexProxy = null;
this.proxyAgent = null;
}
@@ -244,16 +256,72 @@ export class EmexService {
* emexdwc.ae serves Vehicles.aspx, QuickGroups.aspx, and QuickDetails.aspx
* without requiring authentication cookies.
*/
/** Build a fresh proxy agent on a random port from the pool (null if proxy off). */
private newProxyAgent(): ProxyAgent | null {
if (!this.emexProxy) return null;
const { host, user, pass, portStart, portEnd } = this.emexProxy;
const port = Math.floor(Math.random() * (portEnd - portStart + 1)) + portStart;
return new ProxyAgent({
uri: `http://${user}:${pass}@${host}:${port}`,
connect: { timeout: 30000 },
requestTls: { timeout: 30000 },
});
}
private async fetchEmexHtml(url: string): Promise<string> {
const res = await fetch(url, {
headers: { "User-Agent": EMEX_UA, Accept: "text/html,application/xhtml+xml" },
signal: AbortSignal.timeout(this.timeout),
...(this.proxyAgent ? { dispatcher: this.proxyAgent } : {}),
} as RequestInit);
if (!res.ok) {
throw new Error(`EMEX HTTP ${res.status} for ${url}`);
const maxAttempts = this.emexProxy ? 3 : 1;
let lastErr: unknown;
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
// Fresh agent every attempt: a single sticky DataImpulse port flakes
// intermittently ("fetch failed"/reset), silently dropping real emex hits
// (~42% blip rate observed — undecoded-vin-rca.md, EMEX #1). A new agent
// rotates the port and forces a new socket even on a single-port pool.
const agent = this.newProxyAgent();
try {
const res = await fetch(url, {
headers: { "User-Agent": EMEX_UA, Accept: "text/html,application/xhtml+xml" },
signal: AbortSignal.timeout(this.timeout),
...(agent ? { dispatcher: agent } : {}),
} as RequestInit);
if (!res.ok) {
throw new Error(`EMEX HTTP ${res.status} for ${url}`);
}
return await res.text();
} catch (err) {
lastErr = err;
const e = err as Error & { cause?: unknown };
// A real HTTP response ("EMEX HTTP 404") is a definitive answer — never retry.
const httpAnswer = /^EMEX HTTP \d/.test(e.message);
const transient =
!httpAnswer &&
(e.name === "TimeoutError" ||
e.name === "AbortError" ||
e.name === "TypeError" ||
/fetch failed|ECONNRESET|ECONNREFUSED|EAI_AGAIN|socket hang up|other side closed|terminated|UND_ERR/i.test(
`${e.message} ${String(e.cause ?? "")}`,
));
if (!transient || attempt === maxAttempts) break;
this.logger.warn(
`EMEX fetch transient error (attempt ${attempt}/${maxAttempts}) for ${url}: ${e.message} — retrying with fresh proxy`,
);
await new Promise((r) => setTimeout(r, 300 * attempt));
}
}
return res.text();
// Last resort (opt-in via EMEX_DIRECT_FALLBACK): one direct, proxy-less attempt
// for when the whole pool is down. Off by default — it exposes the origin IP.
if (this.emexProxy && this.emexDirectFallback) {
try {
const res = await fetch(url, {
headers: { "User-Agent": EMEX_UA, Accept: "text/html,application/xhtml+xml" },
signal: AbortSignal.timeout(this.timeout),
} as RequestInit);
if (res.ok) return await res.text();
throw new Error(`EMEX HTTP ${res.status} for ${url}`);
} catch (err) {
lastErr = err;
}
}
throw lastErr;
}
/**
@@ -735,9 +803,7 @@ export class EmexService {
);
return httpResult;
}
this.logger.log(
"Plain-HTTP path returned 0 parts; falling back to Playwright scraper",
);
this.logger.log("Plain-HTTP path returned 0 parts; falling back to Playwright scraper");
} catch (err) {
this.logger.warn(
`Plain-HTTP path failed: ${(err as Error).message}; falling back to Playwright`,
@@ -797,9 +863,7 @@ export class EmexService {
throw new Error("No Unit.aspx link in QuickDetails response");
}
const unitRel = unitMatch[1].replace(/&amp;/g, "&");
const unitUrl = unitRel.startsWith("http")
? unitRel
: new URL(unitRel, categoryUrl).toString();
const unitUrl = unitRel.startsWith("http") ? unitRel : new URL(unitRel, categoryUrl).toString();
// 2) Unit.aspx — main extraction target
const unitHtml = await this.fetchEmexHtml(unitUrl);
@@ -853,9 +917,13 @@ export class EmexService {
.trim();
for (const trMatch of html.matchAll(trRx)) {
const body = trMatch[1];
const oem = stripTags(body.match(/<td\b[^>]*\bname="c_oem"[^>]*>([\s\S]*?)<\/td>/)?.[1] ?? "");
const oem = stripTags(
body.match(/<td\b[^>]*\bname="c_oem"[^>]*>([\s\S]*?)<\/td>/)?.[1] ?? "",
);
if (!oem) continue;
const pnc = stripTags(body.match(/<td\b[^>]*\bname="c_pnc"[^>]*>([\s\S]*?)<\/td>/)?.[1] ?? "");
const pnc = stripTags(
body.match(/<td\b[^>]*\bname="c_pnc"[^>]*>([\s\S]*?)<\/td>/)?.[1] ?? "",
);
const name = stripTags(
body.match(/<td\b[^>]*\bname="c_name"[^>]*>([\s\S]*?)<\/td>/)?.[1] ?? "",
);
@@ -929,11 +997,7 @@ export class EmexService {
return { width: buf.readUInt16LE(6), height: buf.readUInt16LE(8) };
}
// PNG — width at byte 16 BE, height at byte 20 BE.
if (
buf.length >= 24 &&
buf[0] === 0x89 &&
buf.slice(1, 4).toString("ascii") === "PNG"
) {
if (buf.length >= 24 && buf[0] === 0x89 && buf.slice(1, 4).toString("ascii") === "PNG") {
return { width: buf.readUInt32BE(16), height: buf.readUInt32BE(20) };
}
return { width: 0, height: 0 };

View File

@@ -1,15 +1,10 @@
import {
BadRequestException,
Inject,
Injectable,
Logger,
NotFoundException,
} from "@nestjs/common";
import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from "@nestjs/common";
import { isValidVin } from "@sase/shared";
import { eq } from "drizzle-orm";
import { DATABASE, type Database } from "../database/database.provider";
import { userVehicles, vehicles } from "../database/schema/core";
import { RedisService } from "../redis/redis.service";
import { vinResolveCacheKeys } from "../vehicles/vin-cache-keys";
@Injectable()
export class InternalVehiclesService {
@@ -21,17 +16,19 @@ export class InternalVehiclesService {
) {}
/**
* Clear all Süper Panel-visible Redis keys for a VIN:
* - vin:resolve:<vin> (positive decode cache)
* - vin:resolve:neg:<vin> (negative cache)
* - vin:lock:<vin> (in-flight decode lock)
* Clear all Süper Panel-visible Redis keys for a VIN (version-namespaced via
* vinResolveCacheKeys so the buster always targets the live decode-chain keys):
* - vin:resolve:<ver>:<vin> (positive decode cache)
* - vin:resolve:neg:<ver>:<vin> (negative cache)
* - vin:lock:<vin> (in-flight decode lock)
* Safe to call when no keys exist (returns 0).
*/
async clearCache(input: { vin: string; reason: string; founderId: string }) {
const vin = input.vin.toUpperCase();
if (!isValidVin(vin)) throw new BadRequestException("Geçersiz VIN");
const keys = [`vin:resolve:${vin}`, `vin:resolve:neg:${vin}`, `vin:lock:${vin}`];
const { cacheKey, negKey, lockKey } = vinResolveCacheKeys(vin);
const keys = [cacheKey, negKey, lockKey];
const existed: string[] = [];
for (const k of keys) {
if (await this.redis.exists(k)) existed.push(k);
@@ -85,7 +82,8 @@ export class InternalVehiclesService {
await this.db.delete(vehicles).where(eq(vehicles.id, existing.id));
// Best-effort Redis cleanup so a re-decode starts fresh.
for (const k of [`vin:resolve:${vin}`, `vin:resolve:neg:${vin}`, `vin:lock:${vin}`]) {
const { cacheKey, negKey, lockKey } = vinResolveCacheKeys(vin);
for (const k of [cacheKey, negKey, lockKey]) {
await this.redis.del(k).catch(() => {});
}

View File

@@ -4,6 +4,7 @@ import { VehiclesService } from "./vehicles.service";
vi.mock("@sase/shared", () => ({
isValidVin: vi.fn(),
getBrandFromWmi: vi.fn().mockReturnValue(null),
}));
import { isValidVin } from "@sase/shared";
@@ -602,4 +603,55 @@ describe("VehiclesService", () => {
await expect(service.deleteVehicle("nonexistent", "u1")).rejects.toThrow(NotFoundException);
});
});
// ─── Q3: previewVin must never return an empty-success on multi-candidate ───
describe("previewVin multi-candidate brand (Q3)", () => {
it("derives a brand for a multi-candidate pcat decode (no all-null result)", async () => {
vi.mocked(isValidVin).mockReturnValue(true);
const { service, partsCatalogsService, pl24Service } = createService({ _selectRows: [] });
// pcat returns >1 car (the empty-200 trigger): siblings share catalogId 'fiat'.
partsCatalogsService.decodeVin.mockResolvedValue({
cars: [
{ id: "1", catalogId: "fiat", name: "EGEA", parameters: [] },
{ id: "2", catalogId: "fiat", name: "TIPO", parameters: [] },
],
});
pl24Service.decodeVin.mockResolvedValue(null);
const res = await service.previewVin("NM435600006H43436");
expect(res.source).toBe("parts-catalogs");
expect(res.brandName).toBe("Fiat"); // was null before the fix
});
});
// ─── Q2: PL24 circuit breaker must only count transient transport faults ───
describe("PL24 circuit breaker fault classification (Q2)", () => {
it("does NOT trip on a definitive upstream negative (non-transient)", async () => {
vi.mocked(isValidVin).mockReturnValue(true);
const { service, partsCatalogsService, pl24Service, redisService } = createService({
_selectRows: [],
});
partsCatalogsService.decodeVin.mockResolvedValue(null); // 0 cars → PL24 joins the race
pl24Service.decodeVin.mockRejectedValue(new Error("HTTP 404: vehicle not found"));
await expect(service.previewVin("WBA00000000000001")).rejects.toThrow(BadRequestException);
// recordPl24Failure() (the only redis.incr caller here) must NOT fire.
expect(redisService.incr).not.toHaveBeenCalled();
});
it("DOES trip on a transient transport error", async () => {
vi.mocked(isValidVin).mockReturnValue(true);
const { service, partsCatalogsService, pl24Service, redisService } = createService({
_selectRows: [],
});
partsCatalogsService.decodeVin.mockResolvedValue(null);
pl24Service.decodeVin.mockRejectedValue(new Error("fetch failed"));
await expect(service.previewVin("WBA00000000000001")).rejects.toThrow(BadRequestException);
expect(redisService.incr).toHaveBeenCalled();
});
});
});

View File

@@ -6,7 +6,7 @@ import {
Logger,
NotFoundException,
} from "@nestjs/common";
import { isValidVin } from "@sase/shared";
import { getBrandFromWmi, isValidVin } from "@sase/shared";
import { Queue } from "bullmq";
import { and, desc, eq, or, sql } from "drizzle-orm";
import { DATABASE, type Database } from "../database/database.provider";
@@ -35,6 +35,7 @@ import { VinApiService } from "../integrations/vin-api/vin-api.service";
import { PrefetchSource } from "../jobs/prefetch.types";
import { CATALOG_PREFETCH_QUEUE } from "../jobs/queues/catalog-prefetch.queue";
import { RedisService } from "../redis/redis.service";
import { vinResolveCacheKeys } from "./vin-cache-keys";
/**
* Per-decode metadata, persisted to `query_logs.timings` jsonb. Despite the
@@ -361,8 +362,13 @@ export class VehiclesService {
throw new BadRequestException("Şase numarası tanınamadı");
}
// Multi-candidate decodes (pcat/emex returned >1 car) have no single model,
// but resolveVin now fills brandName from the first candidate so this is never
// an empty success. WMI map is the final fallback.
const brandName = resolved.brandName ?? getBrandFromWmi(vin.slice(0, 3));
return {
brandName: resolved.brandName,
brandName,
model: resolved.model,
year: resolved.year,
engine: resolved.engine,
@@ -374,7 +380,11 @@ export class VehiclesService {
// VIN decode result is deterministic; cache aggressively. Negatives shorter
// so transient upstream errors don't poison results for a full day.
private static readonly RESOLVE_TTL_POSITIVE_S = 86_400; // 24h
private static readonly RESOLVE_TTL_NEGATIVE_S = 21_600; // 6h
// Negatives expire fast so a transient upstream blip or a freshly-deployed fix
// self-heals within the hour instead of persisting a phantom-undecoded VIN for
// 6h. Combined with DECODE_CHAIN_VERSION namespacing (vin-cache-keys.ts), a
// decode-chain fix invalidates stale negatives immediately on deploy.
private static readonly RESOLVE_TTL_NEGATIVE_S = 1_800; // 30m
private static readonly RESOLVE_LOCK_TTL_S = 60;
private static readonly RESOLVE_WAIT_POLL_MS = 250;
private static readonly RESOLVE_WAIT_TIMEOUT_MS = 30_000;
@@ -448,9 +458,7 @@ export class VehiclesService {
return this.resolveEmexCarByIndex(vin, emexCarIndex);
}
const cacheKey = `vin:resolve:${vin}`;
const negKey = `vin:resolve:neg:${vin}`;
const lockKey = `vin:lock:${vin}`;
const { cacheKey, negKey, lockKey } = vinResolveCacheKeys(vin);
// 1. Cache hits — return immediately.
const cached = await this.redis.getJson<VinResolveResult>(cacheKey);
@@ -697,16 +705,21 @@ export class VehiclesService {
return { kind: "pl24", r: v };
} catch (err) {
if (ctx) ctx.timings.pl24 = Date.now() - pl24Start;
await this.recordPl24Failure();
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;
const isTransient =
e.name === "TimeoutError" ||
/fetch failed|ECONNRESET|ECONNREFUSED|EAI_AGAIN|terminated|UND_ERR|ConnectTimeout|timeout|aborted/i.test(
`${e.message} ${String(e.cause ?? "")}`,
);
// The circuit breaker exists to shed PL24 *latency* (p95 was 12min). Only
// transient transport faults (proxy/network/timeout) count toward it. A
// definitive upstream negative — NotFound/BadRequest "this VIN isn't in
// PL24" — is a real, fast answer, NOT a fault: counting it tripped the
// global breaker after 3 unrelated misses and starved PL24 for every
// subsequent VIN (the sibling-VIN inconsistency in undecoded-vin-rca.md).
if (isTransient) {
await this.recordPl24Failure();
if (outcome) outcome.transient = true;
}
this.logger.warn(`PL24 decode failed for ${vin}: ${e.message}`);
return { kind: "pl24", r: null };
@@ -808,6 +821,10 @@ export class VehiclesService {
const pcatCandidates = pcatResult?.cars && pcatResult.cars.length > 1 ? pcatResult.cars : null;
if (pcatCandidates) {
this.logger.log(`Returning ${pcatCandidates.length} pcat candidates for ${vin}`);
// Derive the brand from the first candidate (they share a catalogId) so the
// multi-candidate path never returns an all-null result. previewVin renders
// this directly; the authenticated picker still uses the full candidate list.
if (!brandName) brandName = this.extractBrandFromPcatCar(pcatCandidates[0]) || null;
return {
brandName,
model: null,
@@ -825,6 +842,9 @@ export class VehiclesService {
if (emexResult?.type === "candidates" && emexResult.candidates.length > 1) {
this.logger.log(`Returning ${emexResult.candidates.length} EMEX candidates for ${vin}`);
// EMEX candidates carry no brand field; fall back to the WMI map so the
// multi-candidate path still names the brand (never an all-null result).
if (!brandName) brandName = getBrandFromWmi(vin.slice(0, 3)) || null;
return {
brandName,
model: null,

View File

@@ -0,0 +1,30 @@
import { describe, expect, it } from "vitest";
import { DECODE_CHAIN_VERSION, vinResolveCacheKeys } from "./vin-cache-keys";
describe("vinResolveCacheKeys (Q4)", () => {
const VIN = "NM435600006H43436";
it("namespaces positive + negative keys by decode-chain version", () => {
const { cacheKey, negKey } = vinResolveCacheKeys(VIN);
expect(cacheKey).toBe(`vin:resolve:${DECODE_CHAIN_VERSION}:${VIN}`);
expect(negKey).toBe(`vin:resolve:neg:${DECODE_CHAIN_VERSION}:${VIN}`);
// The version segment is what invalidates stale negatives on deploy.
expect(cacheKey).toContain(`:${DECODE_CHAIN_VERSION}:`);
expect(negKey).toContain(`:${DECODE_CHAIN_VERSION}:`);
});
it("keeps the in-flight lock key version-independent (cross-deploy dedupe)", () => {
const { lockKey } = vinResolveCacheKeys(VIN);
expect(lockKey).toBe(`vin:lock:${VIN}`);
expect(lockKey).not.toContain(DECODE_CHAIN_VERSION);
});
it("bumping the version changes the namespace (old entries become unreachable)", () => {
// Guards the invariant: a non-empty version prefixes every cache lookup, so a
// version bump can never collide with a prior version's keys.
const { cacheKey, negKey } = vinResolveCacheKeys(VIN);
expect(cacheKey.startsWith("vin:resolve:")).toBe(true);
expect(negKey.startsWith("vin:resolve:neg:")).toBe(true);
expect(DECODE_CHAIN_VERSION.length).toBeGreaterThan(0);
});
});

View File

@@ -0,0 +1,31 @@
/**
* Redis key builders for the VIN resolve cache, namespaced by decode-chain version.
*
* Bump DECODE_CHAIN_VERSION whenever a change to the decode chain (sources,
* routing, dispatch, gating, brand mapping) could turn a previously-cached
* "unknown" into a hit. Bumping instantly invalidates every stale positive and
* negative entry on deploy: without it, the negative cache keeps serving the old
* miss for its full TTL and a real fix looks like it "didn't work" until the
* cache ages out. This was the single biggest source of phantom-undecoded VINs
* (see undecoded-vin-rca.md §2/§6 — ~17 of 124 historical misses already
* decoded but were masked by a stale 6h negative cache).
*
* Both the decode path (VehiclesService) and the admin cache-buster
* (InternalVehiclesService) MUST build keys through here so they stay in sync.
*/
export const DECODE_CHAIN_VERSION = "2";
export function vinResolveCacheKeys(vin: string): {
cacheKey: string;
negKey: string;
lockKey: string;
} {
const v = DECODE_CHAIN_VERSION;
return {
cacheKey: `vin:resolve:${v}:${vin}`,
negKey: `vin:resolve:neg:${v}:${vin}`,
// The in-flight lock is transient (60s) and version-independent — keep it
// un-namespaced so concurrent decoders still dedupe across a deploy boundary.
lockKey: `vin:lock:${vin}`,
};
}