fix(vinpin): clear leftover launch-error modal on acquire give-up + cap relogin
Some checks failed
QA Gate (P0/P1) / Test affected app (pull_request) Has been cancelled
Some checks failed
QA Gate (P0/P1) / Test affected app (pull_request) Has been cancelled
The Renault decode fell to not_found when Rpartstore is DOWN (hard
launch-error modal) even though Dialogys should take over. Root cause:
- acquireLoadedRpartstore's budget-exhausted / never-loaded return-false
paths left the centered launch-error modal on screen (only the flaky
OCR error-detect branch dismissed it). The modal then blocked the
Dialogys grid-return.
- returnToBrandGrid's tab-✕ can't close a centered dialog, so every
"not on brand grid (try N/4)" wedged and re-entered ensureBrandGrid,
which escalated to logoutAndRelogin → close()+launch() ("browser
disconnected — will relaunch") on EVERY iteration, thrashing on a stale
page ref until the 180s budget → not_found.
Fixes (conservative, all safety nets intact):
1. Wrap acquireLoadedRpartstore so EVERY false return runs a best-effort
defensive dismiss (Escape → click launch-error OK 868,530 → Escape),
unconditional of the OCR read. Harmless when no modal is up.
2. returnToBrandGrid + ensureBrandGrid dismiss a possible centered modal
before the tab-✕ close so a leftover dialog can't wedge the loop.
3. Cap the logout+relogin escalation to ONE attempt per decode/warm-up
(reloginUsedThisDecode) — a capped exhaustion poisons the seat for a
clean cold restart instead of looping close()+launch() until budget.
Keeps the OCR fast-path branch, maxOpens/acquireBudgetMs=14s, fcc0298,
61b5769, Fiat path, never-throw/budget/poison all intact.
Tests: +3 (budget-exhausted defensive dismiss; grid-return modal-clear
before tab-✕; relogin capped to one attempt) — 88 vinpin tests green,
tsc + biome clean. Needs prod validation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -479,4 +479,105 @@ describe("VinpinDriverService — Rpartstore spinner guard", () => {
|
||||
raw: { source: "vinpin-dialogys" },
|
||||
});
|
||||
});
|
||||
|
||||
it("acquire give-up (budget-exhausted) DEFENSIVELY dismisses a leftover modal (Escape + launch-error OK) even when the OCR branch never fires", async () => {
|
||||
const driver = new VinpinDriverService();
|
||||
const any = driver as unknown as Record<string, unknown>;
|
||||
// Budget already spent → the inner acquire returns false at its first budget
|
||||
// check WITHOUT ever hitting the OCR error-detect branch (raise/poll not called).
|
||||
const raise = vi.spyOn(any as never, "raiseWarmWindow").mockResolvedValue(true as never);
|
||||
const poll = vi.spyOn(any as never, "pollRpartstoreState");
|
||||
const page = {
|
||||
keyboard: { press: vi.fn(async () => undefined) },
|
||||
mouse: { click: vi.fn(async () => undefined) },
|
||||
waitForTimeout: vi.fn(async () => undefined),
|
||||
};
|
||||
|
||||
const result = await (
|
||||
any.acquireLoadedRpartstore as (
|
||||
p: unknown,
|
||||
c: unknown,
|
||||
d: number,
|
||||
w: boolean,
|
||||
) => Promise<boolean>
|
||||
).call(driver, page, {}, Date.now() - 1, true);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(raise).not.toHaveBeenCalled(); // budget already gone → never opened
|
||||
expect(poll).not.toHaveBeenCalled();
|
||||
// The wrapper's unconditional defensive clear ran on the false path: Escape +
|
||||
// click the centered launch-error OK, so no modal is left to wedge the grid-return.
|
||||
expect(page.keyboard.press).toHaveBeenCalledWith("Escape");
|
||||
expect(page.mouse.click).toHaveBeenCalledWith(
|
||||
VINPIN_COORDS.rpartstoreLaunchErrorOk.x,
|
||||
VINPIN_COORDS.rpartstoreLaunchErrorOk.y,
|
||||
);
|
||||
});
|
||||
|
||||
it("grid-return dismisses a blocking modal BEFORE each tab-✕ close (can't wedge into a relogin thrash)", async () => {
|
||||
const driver = new VinpinDriverService();
|
||||
const any = driver as unknown as Record<string, unknown>;
|
||||
const order: string[] = [];
|
||||
// ocrRegion on a fake page fails-safe to "" (never the brand grid) → the loop runs.
|
||||
const dismiss = vi.spyOn(any as never, "dismissBlockingModal").mockImplementation((async () => {
|
||||
order.push("dismiss");
|
||||
}) as never);
|
||||
vi.spyOn(any as never, "ensureBrandGrid").mockResolvedValue(undefined as never);
|
||||
const page = {
|
||||
mouse: {
|
||||
click: vi.fn(async () => {
|
||||
order.push("click");
|
||||
}),
|
||||
},
|
||||
keyboard: { press: vi.fn(async () => undefined) },
|
||||
waitForTimeout: vi.fn(async () => undefined),
|
||||
};
|
||||
|
||||
await (any.returnToBrandGrid as (p: unknown, c: unknown) => Promise<void>).call(
|
||||
driver,
|
||||
page,
|
||||
{},
|
||||
);
|
||||
|
||||
expect(dismiss).toHaveBeenCalled();
|
||||
// The modal-clear precedes the tab-✕ click on the very first iteration.
|
||||
expect(order[0]).toBe("dismiss");
|
||||
expect(order[1]).toBe("click");
|
||||
expect(page.mouse.click).toHaveBeenCalledWith(
|
||||
VINPIN_COORDS.catalogTabClose.x,
|
||||
VINPIN_COORDS.catalogTabClose.y,
|
||||
);
|
||||
});
|
||||
|
||||
it("caps the logout+relogin escalation to ONE attempt per decode (no close+relaunch thrash)", async () => {
|
||||
const driver = new VinpinDriverService();
|
||||
const any = driver as unknown as Record<string, unknown>;
|
||||
// Fresh decode → relogin budget starts unused.
|
||||
any.reloginUsedThisDecode = false;
|
||||
vi.spyOn(any as never, "submitVinPowerLogin").mockResolvedValue(false as never);
|
||||
vi.spyOn(any as never, "dismissDisconnected").mockResolvedValue(undefined as never);
|
||||
// Relogin itself fails (does NOT relaunch here) — the point is it's only TRIED once.
|
||||
const relogin = vi.spyOn(any as never, "logoutAndRelogin").mockResolvedValue(false as never);
|
||||
const page = {
|
||||
waitForTimeout: vi.fn(async () => undefined),
|
||||
keyboard: { press: vi.fn(async () => undefined) },
|
||||
mouse: { click: vi.fn(async () => undefined) },
|
||||
};
|
||||
const ensureGrid = (p: unknown, c: unknown, allow?: boolean) =>
|
||||
(any.ensureBrandGrid as (p: unknown, c: unknown, a?: boolean) => Promise<void>).call(
|
||||
driver,
|
||||
p,
|
||||
c,
|
||||
allow,
|
||||
);
|
||||
|
||||
// Two grid-returns inside ONE decode (as returnToBrandGrid's loop would do). The
|
||||
// 2nd must NOT re-escalate → no repeated close()+launch() "browser disconnected" loop.
|
||||
await ensureGrid(page, {}, true);
|
||||
await ensureGrid(page, {}, true);
|
||||
|
||||
expect(relogin).toHaveBeenCalledTimes(1); // one relogin only — the 2nd is capped
|
||||
expect(any.reloginUsedThisDecode).toBe(true);
|
||||
expect(any.sessionPoisoned).toBe(true); // capped path poisons for a clean cold restart
|
||||
});
|
||||
});
|
||||
|
||||
@@ -150,6 +150,13 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
* reusing/reconnecting to the poisoned warm session. Survives close() on
|
||||
* purpose (close() resets the session flags, not the poison marker). */
|
||||
private sessionPoisoned = false;
|
||||
/** Guards the `ensureBrandGrid → logoutAndRelogin` escalation to AT MOST ONE
|
||||
* relogin (close+relaunch of the browser) per decode/warm-up. Without it, the
|
||||
* grid-return runs `ensureBrandGrid` on every iteration and each exhausted
|
||||
* attempt re-escalates → close()+launch() thrash ("browser disconnected — will
|
||||
* relaunch" repeatedly) until the wall-clock budget. Reset at the top of each
|
||||
* decode and each warm-up; set the first time a relogin is attempted. */
|
||||
private reloginUsedThisDecode = false;
|
||||
|
||||
// ─── Warm-session daemon state ───────────────────────────
|
||||
/** True once warmUp() has launched + logged in + opened the Fiat ePER, Renault
|
||||
@@ -270,6 +277,8 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
cfg: VinpinConfig,
|
||||
deadline: number,
|
||||
): Promise<VinpinDecodeResult | null> {
|
||||
// Fresh relogin budget for THIS decode (one close+relaunch escalation, no more).
|
||||
this.reloginUsedThisDecode = false;
|
||||
// A prior decode left the seat unrecoverable → never reuse/reconnect to it;
|
||||
// tear it down so this decode cold re-establishes from a fresh browser.
|
||||
if (this.sessionPoisoned) {
|
||||
@@ -432,6 +441,8 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
if (!cfg.enabled || !cfg.warmDaemon || !cfg.user || !cfg.pass) return false;
|
||||
this.warm = false;
|
||||
this.taskbarCoords = {};
|
||||
// Fresh relogin budget for this establish (one close+relaunch escalation, no more).
|
||||
this.reloginUsedThisDecode = false;
|
||||
try {
|
||||
await this.close(); // always warm from a fresh browser
|
||||
await this.launch(cfg);
|
||||
@@ -1033,6 +1044,11 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
// the browser-tab ✕ (93,45). The old windowClose (1298,14) actually hits the
|
||||
// HTML-Access language selector (opens a dropdown), so it's no longer used here.
|
||||
this.logger.warn(`not on brand grid (try ${i + 1}/4) — closing open window`);
|
||||
// A leftover CENTERED dialog (most often the Rpartstore launch-error modal the
|
||||
// acquire give-up can leave up) blocks the grid AND can't be closed by the
|
||||
// tab-✕, so it would wedge every try and escalate into a relogin-disconnect
|
||||
// thrash. Clear it FIRST (best-effort; harmless when nothing is up).
|
||||
await this.dismissBlockingModal(page);
|
||||
await page.mouse.click(VINPIN_COORDS.catalogTabClose.x, VINPIN_COORDS.catalogTabClose.y);
|
||||
await page.waitForTimeout(VINPIN_WAITS.afterWindowClose);
|
||||
// Dismiss an accidentally-opened dropdown/confirm before re-OCR.
|
||||
@@ -1201,6 +1217,10 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
// the grid. Window-close first, then the browser-tab ✕ as a fallback.
|
||||
if (VINPIN_OCR.catalogWindowOpen.test(t)) {
|
||||
this.logger.log(`catalog window resumed open — closing to reach grid (try ${i + 1}/6)`);
|
||||
// A centered launch-error/confirm dialog can ride OVER the resumed catalog and
|
||||
// survive the tab-✕ (it's not the window chrome) — clear it first so it can't
|
||||
// wedge all 6 tries and force the relogin escalation. Best-effort, harmless.
|
||||
await this.dismissBlockingModal(page);
|
||||
// PRIMARY: the browser-tab ✕ (93,45). The window-close (1298,14) actually
|
||||
// opens the HTML-Access language dropdown, so it's no longer clicked here.
|
||||
await page.mouse.click(VINPIN_COORDS.catalogTabClose.x, VINPIN_COORDS.catalogTabClose.y);
|
||||
@@ -1219,12 +1239,22 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
// into the ePER-open loop on a dirty desktop, escalate ONCE to logout + relogin
|
||||
// (end the RDS session and log back in to a fresh grid). One-shot: the inner
|
||||
// relogin runs ensureBrandGrid with allowRelogin=false, so this can't recurse.
|
||||
if (allowRelogin && (await this.logoutAndRelogin(cfg))) {
|
||||
this.sessionPoisoned = false; // relogin reached a fresh, confirmed-clean grid
|
||||
return;
|
||||
//
|
||||
// CAP: at most ONE relogin per decode/warm-up (`reloginUsedThisDecode`). This
|
||||
// method is re-entered on every returnToBrandGrid iteration; without the cap a
|
||||
// modal-blocked grid re-escalates each time → close()+launch() thrash ("browser
|
||||
// disconnected — will relaunch" in a loop) until the wall-clock budget → not_found.
|
||||
// Marked used BEFORE the attempt so a failed relogin can't be re-tried either.
|
||||
if (allowRelogin && !this.reloginUsedThisDecode) {
|
||||
this.reloginUsedThisDecode = true;
|
||||
if (await this.logoutAndRelogin(cfg)) {
|
||||
this.sessionPoisoned = false; // relogin reached a fresh, confirmed-clean grid
|
||||
return;
|
||||
}
|
||||
}
|
||||
// Relogin disabled (already the one-shot attempt) or it too failed → poison the
|
||||
// seat so the next decode cold re-establishes rather than reusing this state.
|
||||
// Relogin disabled, already used this decode, or it too failed → poison the seat
|
||||
// so the next decode cold re-establishes rather than reusing this state (and, when
|
||||
// the cap is hit, the caller's own budget path ends cleanly instead of thrashing).
|
||||
this.sessionPoisoned = true;
|
||||
this.logger.warn("VinPower brand grid not confirmed (relogin exhausted) — poisoning seat");
|
||||
}
|
||||
@@ -1381,6 +1411,25 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
cfg: VinpinConfig,
|
||||
deadline: number,
|
||||
warm: boolean,
|
||||
): Promise<boolean> {
|
||||
const loaded = await this.acquireLoadedRpartstoreInner(page, cfg, deadline, warm);
|
||||
// DEFENSIVE CLEAR on EVERY give-up: whenever the acquire returns false (budget
|
||||
// exhausted, never-loaded after maxOpens, OR the OCR error-detect branch), make
|
||||
// sure no launch-error modal is left blocking the grid before the caller runs the
|
||||
// Dialogys grid-return. The OCR error-detect branch is a fast-path bonus but is
|
||||
// unreliable (didn't fire on a DOWN Rpartstore live), and the budget/never-loaded
|
||||
// paths dismiss nothing — a leftover centered modal then wedges every
|
||||
// returnToBrandGrid try and thrashes into a relogin-disconnect loop until the
|
||||
// wall-clock budget. This dismiss is harmless when no modal is up.
|
||||
if (!loaded) await this.dismissBlockingModal(page);
|
||||
return loaded;
|
||||
}
|
||||
|
||||
private async acquireLoadedRpartstoreInner(
|
||||
page: Page,
|
||||
cfg: VinpinConfig,
|
||||
deadline: number,
|
||||
warm: boolean,
|
||||
): Promise<boolean> {
|
||||
// Sub-cap the acquire loop below the full decode deadline so a hopeless
|
||||
// Rpartstore leaves wall-clock room (within the budget) to reach Dialogys.
|
||||
@@ -1488,6 +1537,33 @@ export class VinpinDriverService implements OnModuleDestroy {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort DEFENSIVE dismiss of any modal/dialog that could be blocking the
|
||||
* brand grid — most importantly the Rpartstore HARD launch-error modal, which the
|
||||
* OCR error-detect branch does NOT reliably catch (it didn't fire on a DOWN
|
||||
* Rpartstore live). Presses Escape, clicks the centered modal-OK coordinate
|
||||
* (`rpartstoreLaunchErrorOk`, 868,530), and Escape again. Unlike
|
||||
* `dismissRpartstoreLaunchError` this does NOT gate on an OCR read — it fires
|
||||
* unconditionally so it works even when the (flaky) modal OCR misses. Harmless
|
||||
* when nothing is up: Escape is a no-op and the click lands on empty desktop
|
||||
* between windows. Fully swallows errors / a missing-or-closed page so it can be
|
||||
* called on every acquire give-up and before each grid-return close without ever
|
||||
* throwing. This is what stops a leftover launch-error modal from wedging the
|
||||
* grid-return into a relogin-disconnect thrash.
|
||||
*/
|
||||
private async dismissBlockingModal(page: Page): Promise<void> {
|
||||
try {
|
||||
await page.keyboard?.press("Escape").catch(() => undefined);
|
||||
await page.mouse
|
||||
?.click(VINPIN_COORDS.rpartstoreLaunchErrorOk.x, VINPIN_COORDS.rpartstoreLaunchErrorOk.y)
|
||||
.catch(() => undefined);
|
||||
await page.keyboard?.press("Escape").catch(() => undefined);
|
||||
await page.waitForTimeout?.(VINPIN_WAITS.afterAlertDismiss).catch(() => undefined);
|
||||
} catch {
|
||||
// best-effort — a missing/closed page must never break the give-up path
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Poll the freshly-opened Rpartstore for its load outcome, capped at `capMs`
|
||||
* (== the old fixed wait, so worst-case timing is unchanged). Each iteration:
|
||||
|
||||
Reference in New Issue
Block a user