Skip to content

fix: [TESIS-999021] keep a discard made while a retry of the event is running - #109

Draft
LauAubert wants to merge 1 commit into
masterfrom
TESIS-999021-discard-survives-running-retry
Draft

LauAubert wants to merge 1 commit into
masterfrom
TESIS-999021-discard-survives-running-retry

Conversation

@LauAubert

Copy link
Copy Markdown
Member

Ticket de Jira

https://proyectofinalfrlp.atlassian.net/browse/TESIS-999021

ID provisorio: se reemplaza por la clave real al cargar la card en Jira. Card: cards/021.md. Sale de una auditoría de código (TESIS-89).


Descripción

Webhooks::RetryFailedEvent carga el FailedEvent, lo reproduce y al terminar escribe el resultado sin releerlo. Si mientras tanto un operador lo descartaba (POST /failed-events/:id/discard), un intento fallido escribía encima: el evento volvía a pending con un next_retry_at y el barrido lo seguía reintentando, aunque lo hubieran sacado de la cola. El spec existente («releases the claim of an event discarded while it was being processed») cubría el descarte, pero no la escritura del worker que seguía corriendo.

  • mark_failed escribe bajo with_lock (relee la fila con FOR UPDATE) y sólo si el evento sigue en processing. Si lo descartaron o lo reencolaron, la decisión del operador gana y el worker no lo toca.
  • mark_succeeded también escribe bajo lock, pero se registra siempre: la venta o el envío ya se procesaron, y succeeded es tan terminal como discarded —no vuelve a la cola— y además dice la verdad.
  • Specs: descarte durante un intento que falla (queda discarded, sin next_retry_at) y durante uno que funciona (queda succeeded).

Evidencia visual

N/A


Cómo probar

El cruce se fija con el spec:

  1. bundle exec rspec spec/poros/webhooks/retry_failed_event_spec.rb → en verde.
  2. Con app/poros/webhooks/retry_failed_event.rb de master, falla «keeps it discarded when the attempt fails»: el evento queda pending.

Verificación: bundle exec rspec (1569 ejemplos, 0 fallas, cobertura 99.89% de línea), rubocop y brakeman limpios.


Impacto y consideraciones

¿Introduce breaking changes?
No

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
No. Un lock de fila al persistir el resultado; el claim atómico del job no cambia.

🤖 Generated with Claude Code

… running

The DLQ worker loads the failed event, replays it and then writes the result
without reading it again. If an operator discarded the event through the API
while the replay ran, a failed attempt wrote over the discard: the event went
back to pending with a next_retry_at and the sweep kept retrying something
the operator had taken out of the queue.

The result is now written under a row lock. A failed attempt leaves the
event alone if it is no longer processing, so the operator's decision wins.
A successful attempt is still recorded: the sale or shipment already went
through, and succeeded is as terminal as discarded while telling the truth.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@TomasMartin2004

Copy link
Copy Markdown
Contributor

Revisado. No lo mergearía para esta entrega.

El bug existe: RetryFailedEvent escribe el resultado sin releer el evento, así que un descarte hecho mientras el reintento corría se pisa, el evento vuelve a pending y el barrido lo sigue reintentando aunque el operador lo haya sacado de la cola.

Pero hace falta que coincidan en el tiempo un reintento en vuelo y un descarte manual del mismo evento, y el daño es que un evento descartado se siga reintentando: no se pierde una venta, no se descuenta stock de más, no queda nada inconsistente en la base. Comparado con el resto de la tanda —500 en todos los listados, ventas que se pierden sin señal, stock que no vuelve— esto no entra en la misma categoría.

Para el backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants