Fix-Runde 1: das Zeitfenster gilt nur, wo auch gedrosselt wird

Eine zeitliche Grenze hebelt --tries nicht bloss fuer Rueckstellungen aus,
sondern ganz: Worker::markJobAsFailedIfWillExceedMaxAttempts prueft die
Versuchszahl ausdruecklich nur if (! $job->retryUntil()). Damit galten die drei
Versuche seit dem Takt NIRGENDS mehr — auch nicht auf der Direktspur und auch
nicht bei ausgeschaltetem Notschalter.

Eine Mail, die aus echtem Grund wirft (SMTP tot, PDF-Rendern kippt), wurde
dadurch sechs Stunden lang wiederholt statt nach drei Versuchen abgelegt, und
ohne Pause, weil der Arbeiter ohne --backoff laeuft. Ihr Fehler stand sechs
Stunden lang nicht in failed_jobs, und auf der Direktspur haette eine einzige
giftige Mail sechs Stunden lang die Kennwort-Zuruecksetzungen blockiert, sobald
Aufgabe 4 den Arbeiter in Prioritaetsreihenfolge lesen laesst.

retryUntil() haengt jetzt an derselben Bedingung wie die Drossel, beide lesen
gedrosselteSpur(). Vorher wird die Elternklasse gefragt: SendQueuedMailable
reicht an die Mailklasse weiter, und die Ueberschreibung nahm das still weg.

Dazu zwei kleinere Punkte aus derselben Durchsicht: der Kopfkommentar behauptete
weiter, eine middleware() auf der Mailklasse lese niemand — widerlegt am
Vendor-Code, jetzt steht der echte Grund dort. Und (int) Settings::get() ergab
auch ohne eingetipptes 0 eine Null (gespeicherte null, nichtnumerischer Wert);
eine Untergrenze steht jetzt an der Lesestelle, an der der Verlust passiert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat/versandtakt
nexxo 2026-08-03 16:55:50 +02:00
parent a713401562
commit 0475aad7b0
3 changed files with 281 additions and 45 deletions

View File

@ -4,68 +4,117 @@ namespace App\Jobs;
use App\Services\Mail\MailLane;
use App\Support\Settings;
use DateTimeInterface;
use Illuminate\Mail\SendQueuedMailable;
use Illuminate\Queue\Middleware\RateLimited;
/**
* Der Auftrag, der eine Mail im Takt ihrer Spur verschickt.
*
* `SendQueuedMailable` kennt keine `middleware()` nachgesehen im Framework:
* die Klasse hat handle, backoff, retryUntil, failed, displayName, __clone und
* sonst nichts. Eine `middleware()` auf der MAILKLASSE liest deshalb niemand.
* Der Weg führt über diesen eigenen Auftrag, den `Mailable::newQueuedJob()`
* über den Container erzeugt eine Bindung tauscht ihn für alle Mails aus,
* ohne dass eine einzige Absendestelle sich ändert.
* `SendQueuedMailable` selbst hat keine `middleware()` nachgezählt im
* Framework: handle, backoff, retryUntil, failed, displayName, __clone. Der
* eigene Auftrag ist trotzdem nicht der einzige Weg: eine `middleware()` auf
* der MAILKLASSE wird sehr wohl gelesen, weil `Mailable::newQueuedJob()` sie
* einsammelt und per `->through()` an den Auftrag weiterreicht.
*
* Der Takt sitzt hier statt im Trait, weil er dann auch für Mailklassen greift,
* die den Trait NICHT benutzen (Pakete, Fortify) und weil er sich mit einer
* Mailable-eigenen `middleware()` verträgt, statt sie zu verdrängen: die
* Bindung im Container tauscht den Auftrag für jede Mail aus, ohne dass eine
* einzige Absendestelle sich ändert.
*
* Zur Falle mit den Versuchen: der Arbeiter läuft mit --tries=3, und eine
* gedrosselte Rückstellung zählt als Versuch. Deshalb `retryUntil()` statt
* eines Versuchszählers eine zeitliche Grenze kennt keine Rückstellungen,
* sondern nur ein Ende.
* eines Versuchszählers aber nur dort, wo wirklich gedrosselt wird.
*/
class PacedMail extends SendQueuedMailable
{
/** @return array<int, object> */
public function middleware(): array
{
$spur = $this->gedrosselteSpur();
return $spur === null ? [] : [new RateLimited($spur)];
}
/**
* Sechs Stunden statt eines Versuchszählers auf den gedrosselten Spuren.
*
* Der Arbeiter prüft `retryUntil` VOR `--tries`: liegt sie in der Zukunft,
* steigt er aus, bevor er `attempts()` gegen die Versuchszahl hält. Genau
* das wird hier gebraucht, denn ein Zähler könnte nicht zwischen „dreimal
* zurückgelegt" und „dreimal gescheitert" unterscheiden die Rechnung, die
* am Abend an der Drossel ansteht, wäre nach dem dritten Mal endgültig
* gescheitert statt verschickt.
*
* Und genau deshalb hängt die Grenze an derselben Bedingung wie die
* Drossel. Ein Zeitfenster hebelt `--tries` nicht bloß für Rückstellungen
* aus, sondern GANZ: `Worker::markJobAsFailedIfWillExceedMaxAttempts`
* prüft die Versuchszahl ausdrücklich nur `if (! $job->retryUntil())`. Wo
* nie gedrosselt wird Direktspur, Notschalter aus , gibt es keine
* Rückstellungen abzufangen, wohl aber eine Mail, die aus echtem Grund
* wirft: die würde sechs Stunden lang wiederholt statt nach drei Versuchen
* abgelegt. Ohne Pause obendrein, weil der Arbeiter ohne `--backoff` läuft
* und `Worker::calculateBackoff` dann auf null Sekunden fällt. Ihr Fehler
* stünde sechs Stunden lang nicht in `failed_jobs` und wäre für jede
* Alarmierung nicht vorhanden; auf der Direktspur zöge der Arbeiter sie
* obendrein bei jedem Durchlauf zuerst sechs Stunden lang keine
* Kennwort-Zurücksetzung.
*
* Sechs Stunden ist weit genug, dass auch ein Lauf über die ganze Nacht
* durchkommt, und eng genug, dass eine Mail, die dann noch nicht draußen
* ist, nicht am nächsten Tag zwischen den neuen auftaucht.
*
* Ohne eigene Rückgabeangabe, wie die Elternklasse: die lässt neben einem
* Zeitpunkt auch einen Zeitstempel zu (`Queue::getJobExpiration` behandelt
* beides), und eine engere Angabe hier zerbräche eine Mailklasse, die von
* diesem Spielraum Gebrauch macht.
*
* @return \DateTimeInterface|int|null
*/
public function retryUntil()
{
// Die Mailklasse zuerst: die Elternklasse reicht an
// `$this->mailable->retryUntil` weiter. Heute deklariert das keine der
// vierzehn — aber eine Überschreibung, die nicht zurückfragt, nähme
// der nächsten ihre Entscheidung still weg.
if (($eigene = parent::retryUntil()) !== null) {
return $eigene;
}
return $this->gedrosselteSpur() === null ? null : now()->addHours(6);
}
/**
* Die Spur, deren Drossel für diesen Auftrag gilt oder null.
*
* Eine Stelle für beide Fragen, weil `middleware()` und `retryUntil()`
* sonst auseinanderlaufen könnten. Beide Richtungen tun weh: ein
* Zeitfenster ohne Drossel nimmt die Versuchszahl weg, ohne etwas dafür zu
* geben, und eine Drossel ohne Zeitfenster ist genau die Falle, um die es
* in dieser Aufgabe geht.
*/
private function gedrosselteSpur(): ?string
{
if (! Settings::bool('mail.pace.enabled', true)) {
return [];
return null;
}
// Die Schlange am Mailable, nicht `MailLane::for(...)`: das hier ist
// der Name, den `Mailable::queue()` an `pushOn()` gegeben hat — die
// Spur, in der dieser Auftrag TATSÄCHLICH liegt. `MailLane::for(...)`
// läge zwischen Einreihen und Abarbeiten eine Zuordnung später: würde
// der Betreiber die Rechnung von ruhig auf wichtig schieben, während
// zweihundert davon in der ruhigen Schlange warten, drosselte sie ab
// diesem Moment das falsche Kontingent.
// läse die Einstellungen erst im Arbeiter: würde der Betreiber die
// Rechnung von ruhig auf wichtig schieben, während zweihundert davon
// in der ruhigen Schlange warten, drosselte sie ab diesem Moment das
// falsche Kontingent.
//
// `?? null` statt eines nackten Zugriffs, weil `$this->mailable` nur
// die Schnittstelle zusichert: eine Mailklasse, die den Trait nicht
// benutzt, hat die Eigenschaft womöglich gar nicht — und liegt dann
// auch in keiner Spur, gehört also nicht gedrosselt.
return match ($this->mailable->queue ?? null) {
MailLane::URGENT => [new RateLimited(MailLane::URGENT)],
MailLane::CALM => [new RateLimited(MailLane::CALM)],
default => [],
MailLane::URGENT => MailLane::URGENT,
MailLane::CALM => MailLane::CALM,
default => null,
};
}
/**
* Sechs Stunden statt eines Versuchszählers.
*
* Der Arbeiter prüft `retryUntil` VOR `--tries`: liegt sie in der Zukunft,
* steigt er aus, bevor er `attempts()` gegen die Versuchszahl hält. Ein
* Zähler dagegen könnte nicht zwischen „dreimal zurückgelegt" und „dreimal
* gescheitert" unterscheiden — die Rechnung, die am Abend zweihundertmal an
* der Drossel ansteht, wäre nach dem dritten Mal endgültig gescheitert.
*
* Weit genug, dass auch ein Lauf über die ganze Nacht durchkommt, und eng
* genug, dass eine Mail, die nach sechs Stunden noch nicht draußen ist,
* nicht am nächsten Tag zwischen den neuen auftaucht.
*/
public function retryUntil(): DateTimeInterface
{
return now()->addHours(6);
}
}

View File

@ -37,13 +37,34 @@ class MailPaceServiceProvider extends ServiceProvider
// läuft im Arbeiter je Auftrag, und eine Änderung des Betreibers wirkt
// damit ab dem nächsten Auftrag statt erst ab dem nächsten Neustart.
RateLimiter::for(MailLane::URGENT, fn () => Limit::perMinutes(
(int) Settings::get('mail.pace.urgent.minutes', 5),
(int) Settings::get('mail.pace.urgent.count', 30),
$this->mindestensEins('mail.pace.urgent.minutes', 5),
$this->mindestensEins('mail.pace.urgent.count', 30),
));
RateLimiter::for(MailLane::CALM, fn () => Limit::perMinutes(
(int) Settings::get('mail.pace.calm.minutes', 10),
(int) Settings::get('mail.pace.calm.count', 20),
$this->mindestensEins('mail.pace.calm.minutes', 10),
$this->mindestensEins('mail.pace.calm.count', 20),
));
}
/**
* Eine Zahl aus den Einstellungen, mindestens eins.
*
* Die Vorgabe in `Settings::get()` greift nur bei FEHLENDER Zeile. Eine
* gespeicherte `null` oder ein Wort statt einer Zahl kommt als Wert
* durch und wird von `(int)` zu 0, ohne dass jemand 0 eingetippt hätte.
* Ein Kontingent von 0 hieße: `tooManyAttempts` ist immer wahr, jede Mail
* wird endlos zurückgelegt und nach sechs Stunden gesammelt zu
* `failed_jobs` genau der Ausgang, den dieser Takt verhindern soll, nur
* über einen anderen Weg. Ein Fenster von 0 Minuten hieße umgekehrt: gar
* keine Drossel.
*
* Die Untergrenze steht hier, weil der Verlust hier passiert. Was ein
* eingetipptes 0 bedeuten SOLL, ist eine Frage an die Eingabe und gehört
* an die Konsolenseite nicht an diese Lesestelle.
*/
private function mindestensEins(string $key, int $default): int
{
return max(1, (int) Settings::get($key, $default));
}
}

View File

@ -9,6 +9,7 @@ use App\Support\Settings;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Mail\Mailable;
use Illuminate\Mail\Mailables\Attachment;
use Illuminate\Mail\Mailables\Content;
use Illuminate\Mail\Mailables\Envelope;
use Illuminate\Queue\Events\JobProcessing;
@ -48,6 +49,62 @@ class TaktProbeMail extends Mailable implements ShouldQueue
}
}
/**
* Eine Mail, die aus echtem Grund kippt nicht wegen der Drossel.
*
* Der Anhang, weil genau dort der teuerste echte Fehler dieses Projekts sitzt:
* `InvoiceMail::attachments()` rendert das PDF beim Senden. Kippt das, ist der
* Auftrag nicht gedrosselt, sondern kaputt, und gehört nach drei Versuchen ins
* Fehlerprotokoll statt sechs Stunden lang wiederholt.
*/
class GiftigeDirektMail extends Mailable implements ShouldQueue
{
use Queueable, RidesALane, SerializesModels;
public function envelope(): Envelope
{
return new Envelope(from: 'takt@example.test', subject: 'Giftig');
}
public function content(): Content
{
return new Content(htmlString: '<p>Giftig</p>');
}
/** @return array<int, Attachment> */
public function attachments(): array
{
throw new RuntimeException('Das Rendern des Anhangs ist gekippt.');
}
}
/**
* Eine Mailklasse, die ihre eigene zeitliche Grenze mitbringt.
*
* Heute tut das keine der vierzehn. Aber `SendQueuedMailable::retryUntil()`
* reicht an die Mailklasse weiter, und eine Überschreibung, die das still
* wegnimmt, würde die nächste ohne einen Hinweis übergehen.
*/
class EigeneGrenzeMail extends Mailable implements ShouldQueue
{
use Queueable, RidesALane, SerializesModels;
public function retryUntil(): DateTimeInterface
{
return now()->addMinutes(11);
}
public function envelope(): Envelope
{
return new Envelope(from: 'takt@example.test', subject: 'Eigene Grenze');
}
public function content(): Content
{
return new Content(htmlString: '<p>Eigene Grenze</p>');
}
}
/**
* Der Auftrag, so wie ihn das Einreihen erzeugt.
*
@ -80,6 +137,23 @@ function drosselName(RateLimited $middleware): string
return (new ReflectionProperty($middleware, 'limiterName'))->getValue($middleware);
}
/** Ein Durchlauf des echten Arbeiters, so wie er im Betrieb läuft. */
function einDurchlaufDesArbeiters(): void
{
Artisan::call('queue:work', [
'--once' => true,
'--tries' => 3,
'--sleep' => 0,
'--queue' => implode(',', [MailLane::DIRECT, MailLane::URGENT, MailLane::CALM]),
]);
}
/** Die Nutzlast des einen Auftrags, der gerade in der Schlange liegt. */
function nutzlastDesAuftrags(): array
{
return json_decode((string) DB::table('jobs')->value('payload'), true);
}
/**
* Die Drossel darf keine Mail verlieren.
*
@ -134,12 +208,7 @@ it('verliert keine Mail, wenn das Kontingent kleiner ist als der Lauf', function
continue;
}
Artisan::call('queue:work', [
'--once' => true,
'--tries' => 3,
'--sleep' => 0,
'--queue' => implode(',', [MailLane::DIRECT, MailLane::URGENT, MailLane::CALM]),
]);
einDurchlaufDesArbeiters();
}
expect(DB::table('failed_jobs')->count())->toBe(0)
@ -147,6 +216,77 @@ it('verliert keine Mail, wenn das Kontingent kleiner ist als der Lauf', function
->and(max($versuche))->toBeGreaterThan(3);
});
/**
* Die Kehrseite: wo nicht gedrosselt wird, muss `--tries` weiter greifen.
*
* Eine zeitliche Grenze schaltet den Versuchszähler nicht bloß für die Drossel
* ab, sondern ganz: `Worker::markJobAsFailedIfWillExceedMaxAttempts` prüft die
* Versuche ausdrücklich nur `if (! $job->retryUntil())`. Eine Mail, die aus
* ECHTEM Grund wirft, würde damit sechs Stunden lang wiederholt ohne Pause,
* weil der Arbeiter ohne `--backoff` läuft und `calculateBackoff` dann auf null
* Sekunden fällt.
*
* Zwei Schäden: der Fehler ist sechs Stunden lang nicht in `failed_jobs`, also
* für jede Alarmierung nicht vorhanden. Und sobald der Arbeiter die Spuren in
* Prioritätsreihenfolge liest, zieht er diese eine giftige Direkt-Mail bei
* JEDEM Durchlauf zuerst sechs Stunden lang keine Kennwort-Zurücksetzung.
*/
it('lässt eine Direkt-Mail, die aus echtem Grund wirft, nach drei Versuchen scheitern', function () {
MailLane::assign(GiftigeDirektMail::class, MailLane::DIRECT);
config(['queue.default' => 'database']);
Mail::to('kunde@example.test')->queue(new GiftigeDirektMail);
// Ohne Zeitfenster wird der Auftrag beim dritten Versuch abgelegt und die
// Schlange ist leer. Mit Zeitfenster käme er endlos wieder — die Schranke
// bei zehn ist der Unterschied zwischen einem roten Test und einem Test,
// der nie fertig wird.
$durchlaeufe = 0;
while (DB::table('jobs')->where('available_at', '<=', now()->getTimestamp())->exists()) {
if (++$durchlaeufe > 10) {
throw new RuntimeException('Der Auftrag hört nicht auf — die Versuchszahl greift nicht.');
}
einDurchlaufDesArbeiters();
}
expect($durchlaeufe)->toBe(3)
->and(DB::table('failed_jobs')->count())->toBe(1)
->and(DB::table('jobs')->count())->toBe(0);
});
it('gibt einem Auftrag ohne Drossel gar kein Zeitfenster', function (string $aufbau) {
config(['queue.default' => 'database']);
$aufbau === 'direkt'
? MailLane::assign(TaktProbeMail::class, MailLane::DIRECT)
: Settings::set('mail.pace.enabled', false);
Mail::to('kunde@example.test')->queue(new TaktProbeMail(1));
expect(nutzlastDesAuftrags()['retryUntil'])->toBeNull();
})->with(['direkt', 'notschalter aus']);
/**
* Bringt die Mailklasse ihre eigene Grenze mit, gilt ihre.
*
* `SendQueuedMailable::retryUntil()` reicht an `$this->mailable->retryUntil`
* weiter. Eine Überschreibung, die das nicht zurückfragt, nähme der nächsten
* Mailklasse ihre Entscheidung still weg.
*/
it('lässt der Mailklasse ihre eigene zeitliche Grenze', function () {
config(['queue.default' => 'database']);
Mail::to('kunde@example.test')->queue(new EigeneGrenzeMail);
// Auf einer gedrosselten Spur — sonst bewiese der Test nur, dass ohne
// Drossel nichts passiert, und nicht, dass die eigene Grenze vorgeht.
expect(MailLane::for(EigeneGrenzeMail::class))->toBe(MailLane::CALM)
->and(nutzlastDesAuftrags()['retryUntil'])->toBe(now()->addMinutes(11)->getTimestamp());
});
/**
* Was der Arbeiter liest, ist der Auftrag in der Schlange, nicht das Objekt.
*
@ -219,6 +359,32 @@ it('nimmt beide Kontingente aus den Einstellungen', function () {
->and($ruhig->decaySeconds)->toBe(540);
});
/**
* Eine unbrauchbare Einstellung darf kein Kontingent von null ergeben.
*
* Die Vorgabe in `Settings::get()` greift nur bei FEHLENDER Zeile. Eine
* gespeicherte `null` oder ein Wort statt einer Zahl kommt als Wert durch
* und wird von `(int)` zu 0. Ein Kontingent von 0 hieße: `tooManyAttempts` ist
* immer wahr, jede Mail wird endlos zurückgelegt und nach sechs Stunden
* gesammelt zu `failed_jobs` genau der Ausgang, den dieser Takt verhindern
* soll, nur über einen anderen Weg. Ein Fenster von 0 Minuten hieße
* umgekehrt: gar keine Drossel.
*/
it('macht aus einer unbrauchbaren Einstellung kein Kontingent von null', function () {
Settings::set('mail.pace.urgent.count', null);
Settings::set('mail.pace.urgent.minutes', null);
Settings::set('mail.pace.calm.count', 'viele');
Settings::set('mail.pace.calm.minutes', -3);
$wichtig = RateLimiter::limiter(MailLane::URGENT)(null);
$ruhig = RateLimiter::limiter(MailLane::CALM)(null);
expect($wichtig->maxAttempts)->toBe(1)
->and($wichtig->decaySeconds)->toBe(60)
->and($ruhig->maxAttempts)->toBe(1)
->and($ruhig->decaySeconds)->toBe(60);
});
it('taktet ohne Einstellung dreißig je fünf und zwanzig je zehn Minuten', function () {
$wichtig = RateLimiter::limiter(MailLane::URGENT)(null);
$ruhig = RateLimiter::limiter(MailLane::CALM)(null);