hjnormey/citas-getter
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: duplicate detected on retry must STOP entire booking process (not just slot)

Browse Source 
Critical logic bug caught by careful review:

- When 'Cita insertada' is returned on a retry, the cita is ALREADY
  inserted into the database — no further booking action needed, just
  confirm and exit (the comment 'Book it!' was misleading).

- When a duplicate ('ya existente'/'DNI'/'existe') was detected during
  skew retry, the code only broke the inner while loop. Since booked_hora
  was None, 'if booked_hora: break' did NOT trigger, and the outer for
  loop continued to the NEXT slot — potentially booking additional
  slots when a cita for this person already exists.

- The EXCEPTION branch's force-reconnect retry had NO duplicate detection
  at all — it fell through to a generic 'else: log()' for any unexpected
  response, including duplicates.

Fixes:
- Added duplicate_detected flag (initialized False) checked in outer
  loop: 'if booked_hora or duplicate_detected: break'
- Set duplicate_detected = True in ALL three duplicate detection paths:
  (1) initial attempt, (2) skew retry loop, (3) EXCEPTION retry
- Added explicit 'Hora no permitida' and 'Fecha posterior' handling
  in EXCEPTION retry branch (was generic else)
- Removed stale '5x43ms' comment → reflects true '120s 3-phase backoff'
- Clarified comments: 'Cita insertada' = already booked (success),
  not a booking action to perform

Tests: 58 passed (added test_duplicate_detected_flag_stops_outer_loop)
This commit is contained in:
cc-ultrav-1
2026-10-03 22:51:33 +02:00
parent f4f5fe0996
commit 7048f44d67
2 changed files with 44 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
book_final.py
View File

@@ -335,10 +335,12 @@ log(f" Priority order: {PRIORITY}")
log(f" Only trying slots confirmed available: {[h for h in PRIORITY if h in available_slots]}")
# IMPORTANT: This loop fires ONE request at a time.
# It EXITS immediately on success (no more requests after "Cita insertada").
# It EXITS immediately on success — "Cita insertada" means the cita was ALREADY
# inserted into the database, so no further booking action is needed; we just
# confirm and stop.
# Each slot is tried ONCE, with retries ONLY for:
# - "Fecha posterior" (clock skew): EACH slot gets its own independent 3-phase
# backoff (43ms×1.5^n for 6 aggressive, 1.15x gradual to 2.5s, 2s×1.2^n extended
# - "Fecha posterior" (server date not rolled): EACH slot gets its own independent
# 3-phase backoff (43ms×1.5^n first 6, 1.15x gradual to 500ms, 2s×1.2^n extended
# to 120s). WE ARE FIGHTING BOTS — every slot retries until certified taken.
# - "EXCEPTION:" (conn drop): force-reconnect + retry on SAME slot
# - "Hora no permitida": DEFINITIVE certification slot is taken → no retry
@@ -346,7 +348,7 @@ log(f" Only trying slots confirmed available: {[h for h in PRIORITY if h in ava
# a later slot's retries even if earlier slots exhausted theirs.
# Only give up on a slot when: certified taken ("Hora no permitida") or
# time budget exhausted (can't certify further) or duplicate detected.
# Duplicate detection ("ya existente") → STOP immediately
# Duplicate detection ("ya existente"/"DNI"/"existe") at ANY point → STOP immediately.
# This prevents duplicate bookings while tolerating clock/network issues.
attempts = [] # in-memory log of all attempts
@@ -354,8 +356,9 @@ booked_hora = None
booked_result = None
# Each slot tracks skew retry exhaustion independently — NO global skip flag.
# The server date may roll during another slot's retry window, so every
# slot must get its own full 5×43ms chance to hit the midnight transition.
# slot must get its own full 120s three-phase backoff chance to hit midnight.
skew_retries_exhausted = False # diagnostics only (per-slot, not global skip)
duplicate_detected = False # STOP entire booking process if cita already exists
for hora in PRIORITY:
if hora not in available_slots:
@@ -430,8 +433,8 @@ for hora in PRIORITY:
log(f"\n✅ CITA BOOKED (on skew retry {skew_attempt})! Hora: {hora}")
break
elif "ya existente" in result2 or "DNI" in result2 or "existe" in result2:
log(f" → POSSIBLE DUPLICATE on retry: {result2[:200]}")
log(f" ⚠️ STOPPING — cita may already exist!")
log(f" → DUPLICATE on retry: {result2[:200]} — STOPPING")
duplicate_detected = True # signal outer loop to STOP entirely
break
elif result2 == "Hora no permitida":
# ✅ CERTIFIED: slot is genuinely taken — safe to give up THIS slot
@@ -442,13 +445,11 @@ for hora in PRIORITY:
log(f" → Retry returned: {result2[:100]} — moving on")
break
# else: still "Fecha posterior" → NOT certified → keep retrying
if booked_hora:
break
if booked_hora or duplicate_detected:
break # success OR duplicate certifies — do NOT try any more slots
elif "ya existente" in result or "DNI" in result or "existe" in result:
log(f" → POSSIBLE DUPLICATE: {result[:200]}")
log(f" ⚠️ STOPPING — a cita for this person may already exist!")
log(f" Use cancel_cita() to check and clean up if needed.")
log(f" → DUPLICATE: {result[:200]} — STOPPING")
break
elif result.startswith("EXCEPTION:"):
@@ -469,8 +470,16 @@ for hora in PRIORITY:
booked_result = result_retry
log(f"\n✅ CITA BOOKED (on retry after conn error)! Hora: {hora}")
break
elif "ya existente" in result_retry or "DNI" in result_retry or "existe" in result_retry:
log(f" → DUPLICATE after reconnect: {result_retry[:200]} — STOPPING")
duplicate_detected = True
break
elif result_retry.startswith("EXCEPTION:"):
log(f" → Connection retry also failed — moving to next slot")
elif result_retry == "Hora no permitida":
log(f" → Slot {hora} taken after reconnect — moving to next.")
elif result_retry == "Fecha posterior al limite permitido":
log(f" → Retry also hit date limit — moving to next slot")
else:
log(f" → Retry result: {result_retry[:100]}")
except Exception as e:

25
test_book_final.py
View File

@@ -323,10 +323,31 @@ class TestBookingLoopSafety(unittest.TestCase):
with open(SCRIPT_PATH) as f:
source = f.read()
loop_start = source.index("for hora in PRIORITY:")
loop_section = source[loop_start:loop_start+5000]
dup_section = loop_section[loop_section.index("ya existente"):loop_section.index("ya existente")+300]
loop_section = source[loop_start:loop_start+10000]
dup_section = loop_section[loop_section.index("ya existente"):loop_section.index("ya existente")+500]
self.assertIn("break", dup_section)
def test_duplicate_detected_flag_stops_outer_loop(self):
"""A duplicate detected during ANY retry must stop the ENTIRE booking
process — not just the current slot — by setting duplicate_detected=True
and checking it in the outer for-loop break condition."""
with open(SCRIPT_PATH) as f:
source = f.read()
loop_start = source.index("for hora in PRIORITY:")
loop_section = source[loop_start:loop_start+10000]
# Flag must be initialized
self.assertIn("duplicate_detected", source)
# Must be set to True when a duplicate is detected
self.assertIn("duplicate_detected = True", loop_section)
# The outer-loop break must reference duplicate_detected
self.assertIn("if booked_hora or duplicate_detected", source)
# Duplicate detection must exist in ALL three branches:
# initial attempt, skew retry loop, and EXCEPTION retry
self.assertIn("ya existente", source)
# Verify there are at least 2 occurrences of "ya existente" (initial + retry)
count = source.count("ya existente")
self.assertGreaterEqual(count, 2, "duplicate check should exist in multiple branches")
def test_loop_does_not_parallelize(self):
"""No threading, async, or parallel execution."""
with open(SCRIPT_PATH) as f: