sffs-tt-basel/sffs-tt-issues
2
0
Fork 0
Code Issues 114 Pull requests Projects Releases Packages Wiki Activity Actions

Mails erst nach dem Commit verschicken, nicht mitten in der Verarbeitung #122

New issue
Open
opened 2026-10-20 11:10:20 +00:00 by claude · 3 comments
claude commented 2026-10-20 11:10:20 +00:00
Collaborator
Copy link

Gefunden bei der Arbeit an #23 (20.10.2026), am Code nachgeprüft auf main 8ac1b58.

Was heute passiert

Es gibt genau einen Versandpunkt: CaptainNotificationService.deliver verschickt die Mail synchron (mailer.send, Zeile 189). Danach schreibt send den Protokolleintrag (Zeilen 159–170). Alle 12 Aufrufer tun das innerhalb einer @Transactional-Methode, mitten in der fachlichen Verarbeitung:

Aufruf in Methode
TeamMatchResultService:160 resultSubmitted submit
TeamMatchResultService:283 forfeitSubmitted submitForfeit
TeamMatchResultService:330 resultChangedByAssociation correct
TeamMatchResultService:428 forfeitLifted liftForfeit
TeamMatchResultService:462 correctionRequested requestCorrection
TeamMatchProposalService:77, :113, :138, :145, :167 create, accept, clearSchedule, reject, withdrawOptions
TeamMatchResource:182 matchDeleted delete. Die Mail „Begegnung gelöscht“ geht vor dem Löschen (Zeile 183) raus
CaptainReminderService:205 sendReminder run: eine Transaktion über alle Begegnungen, danach die Elo-Rechnung (#23)

Folgen

  • Mail ohne Daten. Scheitert nach dem Versand noch etwas, rollt die Transaktion zurück, aber die Mail ist schon draussen. Das kann ein späterer Schritt sein, ein Constraint beim Commit oder im Job die Elo-Rechnung. Der Captain liest „Termin bestätigt“ oder „Begegnung gelöscht“, und in der Datenbank steht nichts davon.
  • Doppelte Mails. Im Fristen-Job wird mit der Transaktion auch der Protokolleintrag zurückgerollt. Der Job hält die Erinnerung darum für unversandt und schickt sie am nächsten Morgen noch einmal, jeden Tag, bis die Ursache behoben ist.
  • Datenbanksperren während SMTP. Die Transaktion hält ihre Sperren, solange der Mailserver antwortet. Ein langsamer Mailserver bremst die Datenbank.
  • Vermischte Zuständigkeiten. Die Fachservices entscheiden und verschicken zugleich. Das hat keine Spec und keine ADR so festgelegt; es ist mit d0554c6 und d14915a (20.07.) so entstanden.

Vorschlag

  • Die Services entscheiden nur und melden ein Ereignis, zum Beispiel „Resultat eingereicht für Begegnung 17“. Sie kennen den Mailer nicht.
  • Ein eigener Benachrichtiger verschickt nach erfolgreichem Commit, über CDI @Observes(during = TransactionPhase.AFTER_SUCCESS). Wird zurückgerollt, geht keine Mail raus.
  • Das Protokoll schreibt der Benachrichtiger nach dem erfolgreichen Versand in einer eigenen, kurzen Transaktion. Ein gescheiterter Versand bekommt keinen Eintrag und wird, wo es eine Wiederholung gibt (Fristen-Job), beim nächsten Lauf erneut versucht. Das tut der Job heute schon.
  • Der Fristen-Job entscheidet je Begegnung in einer eigenen Transaktion. Seine Erinnerungen laufen über denselben Weg. Darauf baut #23 auf.
  • Eine ADR zur Regel „Mails nach dem Commit, nur der Benachrichtiger kennt den Mailer“, dazu eine Zeile in CLAUDE.md und eine ArchUnit-Regel (#120): Nur der Benachrichtiger hängt von NotificationMailer ab.

Für den Plan

  • Die Reihenfolge der Mails und ihre Texte bleiben unverändert. Es ändert sich nur, wann sie verschickt werden.
  • Bestehende Tests prüfen den Versand vermutlich direkt im Aufruf. Mit AFTER_SUCCESS braucht ein Test eine abgeschlossene Transaktion. Welche Tests das betrifft, wird erst aufgezählt und dann abgestimmt.

Abnahme

  • Ein Test lässt eine Verarbeitung nach dem Versandaufruf scheitern und prüft: keine Mail, kein Protokolleintrag. Heute geht die Mail raus.
  • Der Fristen-Job verschickt nach einem Abbruch keine Erinnerung doppelt.
  • TeamMatchResource.delete: Scheitert das Löschen, geht keine Mail „Begegnung gelöscht“ raus.
  • Kein Fachservice importiert NotificationMailer oder CaptainNotificationService.deliver mehr. ArchUnit prüft das.
Gefunden bei der Arbeit an #23 (20.10.2026), am Code nachgeprüft auf `main` 8ac1b58. ## Was heute passiert Es gibt genau einen Versandpunkt: `CaptainNotificationService.deliver` verschickt die Mail **synchron** (`mailer.send`, Zeile 189). Danach schreibt `send` den Protokolleintrag (Zeilen 159–170). Alle **12 Aufrufer** tun das innerhalb einer `@Transactional`-Methode, mitten in der fachlichen Verarbeitung: | Aufruf | in Methode | |---|---| | `TeamMatchResultService:160` `resultSubmitted` | `submit` | | `TeamMatchResultService:283` `forfeitSubmitted` | `submitForfeit` | | `TeamMatchResultService:330` `resultChangedByAssociation` | `correct` | | `TeamMatchResultService:428` `forfeitLifted` | `liftForfeit` | | `TeamMatchResultService:462` `correctionRequested` | `requestCorrection` | | `TeamMatchProposalService:77`, `:113`, `:138`, `:145`, `:167` | `create`, `accept`, `clearSchedule`, `reject`, `withdrawOptions` | | `TeamMatchResource:182` `matchDeleted` | `delete`. Die Mail „Begegnung gelöscht“ geht **vor** dem Löschen (Zeile 183) raus | | `CaptainReminderService:205` `sendReminder` | `run`: eine Transaktion über alle Begegnungen, danach die Elo-Rechnung (#23) | ## Folgen - **Mail ohne Daten.** Scheitert nach dem Versand noch etwas, rollt die Transaktion zurück, aber die Mail ist schon draussen. Das kann ein späterer Schritt sein, ein Constraint beim Commit oder im Job die Elo-Rechnung. Der Captain liest „Termin bestätigt“ oder „Begegnung gelöscht“, und in der Datenbank steht nichts davon. - **Doppelte Mails.** Im Fristen-Job wird mit der Transaktion auch der Protokolleintrag zurückgerollt. Der Job hält die Erinnerung darum für unversandt und schickt sie am nächsten Morgen noch einmal, jeden Tag, bis die Ursache behoben ist. - **Datenbanksperren während SMTP.** Die Transaktion hält ihre Sperren, solange der Mailserver antwortet. Ein langsamer Mailserver bremst die Datenbank. - **Vermischte Zuständigkeiten.** Die Fachservices entscheiden und verschicken zugleich. Das hat keine Spec und keine ADR so festgelegt; es ist mit `d0554c6` und `d14915a` (20.07.) so entstanden. ## Vorschlag - **Die Services entscheiden nur und melden ein Ereignis**, zum Beispiel „Resultat eingereicht für Begegnung 17“. Sie kennen den Mailer nicht. - **Ein eigener Benachrichtiger verschickt nach erfolgreichem Commit**, über CDI `@Observes(during = TransactionPhase.AFTER_SUCCESS)`. Wird zurückgerollt, geht keine Mail raus. - **Das Protokoll schreibt der Benachrichtiger nach dem erfolgreichen Versand** in einer eigenen, kurzen Transaktion. Ein gescheiterter Versand bekommt keinen Eintrag und wird, wo es eine Wiederholung gibt (Fristen-Job), beim nächsten Lauf erneut versucht. Das tut der Job heute schon. - **Der Fristen-Job** entscheidet je Begegnung in einer eigenen Transaktion. Seine Erinnerungen laufen über denselben Weg. Darauf baut #23 auf. - Eine ADR zur Regel „Mails nach dem Commit, nur der Benachrichtiger kennt den Mailer“, dazu eine Zeile in CLAUDE.md und eine ArchUnit-Regel (#120): Nur der Benachrichtiger hängt von `NotificationMailer` ab. ## Für den Plan - Die Reihenfolge der Mails und ihre Texte bleiben unverändert. Es ändert sich nur, **wann** sie verschickt werden. - Bestehende Tests prüfen den Versand vermutlich direkt im Aufruf. Mit `AFTER_SUCCESS` braucht ein Test eine abgeschlossene Transaktion. Welche Tests das betrifft, wird erst aufgezählt und dann abgestimmt. ## Abnahme - Ein Test lässt eine Verarbeitung nach dem Versandaufruf scheitern und prüft: keine Mail, kein Protokolleintrag. Heute geht die Mail raus. - Der Fristen-Job verschickt nach einem Abbruch keine Erinnerung doppelt. - `TeamMatchResource.delete`: Scheitert das Löschen, geht keine Mail „Begegnung gelöscht“ raus. - Kein Fachservice importiert `NotificationMailer` oder `CaptainNotificationService.deliver` mehr. ArchUnit prüft das.
claude commented 2026-10-20 13:08:52 +00:00
Author
Collaborator
Copy link

Aus dem Review von ADR 0033 (Session „reviewer“, 20.10.2026), am Code bestätigt auf main d40e99d. Das muss beim Bau dieses Issues entschieden werden.

Der Protokolleintrag ist nicht nur Protokoll

CaptainReminderService.checkAutoConfirmation (Zeile 188) rechnet die Frist für die automatische Bestätigung ab dem neuesten Eintrag RESULTAT_EINGEREICHT (sentAt). Fehlt er, nimmt der Job ersatzweise match.scheduledAt.

Schreibt der Benachrichtiger den Eintrag nach dem Commit und scheitert dabei, beginnt die Frist am Spieltag statt am Tag der Einreichung, und die Begegnung wird zu früh automatisch bestätigt. Der Eintrag hat heute zwei Bedeutungen: Versandnachweis und Einreichungszeitpunkt (CLAUDE.md, „Eine Bedeutung je Feld“).

Zwei Wege, beide brauchen Ollis Entscheid:

  • (a) Der Einreichungszeitpunkt bekommt ein eigenes Feld an der Begegnung und wird in der Haupttransaktion geschrieben. Der Eintrag bleibt reines Protokoll nach dem Commit. Das ist ein neues Feld, also nur mit Ollis Ja.
  • (b) Der Eintrag wird weiter in der Haupttransaktion geschrieben, nur der Versand läuft nach dem Commit. sentAt heisst dann „beauftragt“, nicht „versandt“.

Verhaltensänderung, die in die Abnahme gehört

Heute wirft ein Versandfehler bei Ereignis-Mails (isMailFailureTolerated=false, CaptainNotificationService.deliver Zeile 191) und rollt den ganzen Vorgang zurück. Fällt der Mailserver aus, kann niemand ein Resultat einreichen. Nach dem Umbau geht die Einreichung durch, nur die Mail fehlt. Ereignis-Mails holt niemand nach; nur die drei Erinnerungen prüft der Fristen-Job über das Protokoll (remind, Zeile 202).

Rahmen: ADR 0033 (docs/adr/0033-schichten-mit-fachkern-und-entitaeten-ohne-active-record.md), Regel 8 („Folgen werden gemeldet, nicht miterledigt“).

Aus dem Review von ADR 0033 (Session „reviewer“, 20.10.2026), am Code bestätigt auf `main` d40e99d. Das muss beim Bau dieses Issues entschieden werden. ## Der Protokolleintrag ist nicht nur Protokoll `CaptainReminderService.checkAutoConfirmation` (Zeile 188) rechnet die Frist für die automatische Bestätigung ab dem neuesten Eintrag `RESULTAT_EINGEREICHT` (`sentAt`). Fehlt er, nimmt der Job ersatzweise `match.scheduledAt`. Schreibt der Benachrichtiger den Eintrag nach dem Commit und scheitert dabei, beginnt die Frist am Spieltag statt am Tag der Einreichung, und die Begegnung wird zu früh automatisch bestätigt. Der Eintrag hat heute zwei Bedeutungen: Versandnachweis und Einreichungszeitpunkt (CLAUDE.md, „Eine Bedeutung je Feld“). Zwei Wege, beide brauchen Ollis Entscheid: - **(a)** Der Einreichungszeitpunkt bekommt ein eigenes Feld an der Begegnung und wird in der Haupttransaktion geschrieben. Der Eintrag bleibt reines Protokoll nach dem Commit. Das ist ein neues Feld, also nur mit Ollis Ja. - **(b)** Der Eintrag wird weiter in der Haupttransaktion geschrieben, nur der Versand läuft nach dem Commit. `sentAt` heisst dann „beauftragt“, nicht „versandt“. ## Verhaltensänderung, die in die Abnahme gehört Heute wirft ein Versandfehler bei Ereignis-Mails (`isMailFailureTolerated=false`, `CaptainNotificationService.deliver` Zeile 191) und rollt den ganzen Vorgang zurück. Fällt der Mailserver aus, kann niemand ein Resultat einreichen. Nach dem Umbau geht die Einreichung durch, nur die Mail fehlt. Ereignis-Mails holt niemand nach; nur die drei Erinnerungen prüft der Fristen-Job über das Protokoll (`remind`, Zeile 202). Rahmen: ADR 0033 (`docs/adr/0033-schichten-mit-fachkern-und-entitaeten-ohne-active-record.md`), Regel 8 („Folgen werden gemeldet, nicht miterledigt“).
claude commented 2026-10-20 16:38:24 +00:00
Author
Collaborator
Copy link

Entscheidung 1 (Olli, 20.10.2026): Weg (a). Der Einreichungszeitpunkt bekommt ein eigenes Feld an der Begegnung und wird beim Einreichen in derselben Transaktion gespeichert. Der Fristen-Job rechnet die automatische Bestätigung nur noch nach diesem Feld. Der Protokolleintrag bleibt reines Versandprotokoll und wird erst nach erfolgreichem Versand geschrieben. Eine Migration übernimmt für bereits eingereichte Begegnungen den heutigen Zeitpunkt aus dem Protokoll.

Verworfen wurde (b): Ein Eintrag beim Speichern hiesse „beauftragt“ statt „versandt“, und eine gescheiterte Erinnerung des Fristen-Jobs würde nie wiederholt.

**Entscheidung 1 (Olli, 20.10.2026): Weg (a).** Der Einreichungszeitpunkt bekommt ein eigenes Feld an der Begegnung und wird beim Einreichen in derselben Transaktion gespeichert. Der Fristen-Job rechnet die automatische Bestätigung nur noch nach diesem Feld. Der Protokolleintrag bleibt reines Versandprotokoll und wird erst nach erfolgreichem Versand geschrieben. Eine Migration übernimmt für bereits eingereichte Begegnungen den heutigen Zeitpunkt aus dem Protokoll. Verworfen wurde (b): Ein Eintrag beim Speichern hiesse „beauftragt“ statt „versandt“, und eine gescheiterte Erinnerung des Fristen-Jobs würde nie wiederholt.
claude commented 2026-10-20 16:40:10 +00:00
Author
Collaborator
Copy link

Entscheidung 2 (Olli, 20.10.2026): Variante (i). Scheitert eine Vorgangs-Mail nach dem Commit, bleibt der Vorgang gespeichert und die Mail ist verloren; der Fehler steht im Server-Log. Nachgeholt werden wie bisher nur die Erinnerungen des Fristen-Jobs. Begründung: Mailserver und Anwendung laufen im selben Rechenzentrum, ein unabhängiger Ausfall nur des Mailservers ist unwahrscheinlich. Heute blockiert ein Mailausfall dagegen die ganze Erfassung. Ein Nachholen auch der Vorgangs-Mails (Outbox) wäre ein eigenes Issue.

**Entscheidung 2 (Olli, 20.10.2026): Variante (i).** Scheitert eine Vorgangs-Mail nach dem Commit, bleibt der Vorgang gespeichert und die Mail ist verloren; der Fehler steht im Server-Log. Nachgeholt werden wie bisher nur die Erinnerungen des Fristen-Jobs. Begründung: Mailserver und Anwendung laufen im selben Rechenzentrum, ein unabhängiger Ausfall nur des Mailservers ist unwahrscheinlich. Heute blockiert ein Mailausfall dagegen die ganze Erfassung. Ein Nachholen auch der Vorgangs-Mails (Outbox) wäre ein eigenes Issue.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
No results found.
Labels
Clear labels   
Kind
Bug

  
Kind
Enhancement

  
Kind
Maintenance
Umbau, Aufraeumen, Tests, Werkzeuge, Absicherung - ohne fachliche Aenderung

  
Priority
P1
Vor der Testphase auf Preview: falsche Rechte, falsche Regel im Captain-Ablauf oder stilles Scheitern

  
Priority
P2
Vor dem Prod-Rollout: im laufenden Betrieb noetig, teils mit festem Termin

  
Priority
P3
Wenn Zeit ist: Darstellung, Komfort, Hygiene

  
Status
In Progress

  
Status
Need More Info

  
Status
Zur Abnahme
In main gemergt und auf Staging; wartet auf Abnahme (Petra bzw. Olli), danach schliessen

  
Änderungen Wettspielreglement

Fehler oder Unklarheit im Wettspielreglement, die der Vorstand im Dokument berichtigen muss

No labels
Kind
Bug
Kind
Enhancement
Kind
Maintenance
Priority
P1
Priority
P2
Priority
P3
Status
In Progress
Status
Need More Info
Status
Zur Abnahme
Änderungen Wettspielreglement
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
sffs-tt-basel/sffs-tt-issues#122
No description provided.