|
@@ -171,8 +171,41 @@ Without the fingerprint, a plugin that filters `woocommerce_mail_callback` to re
|
|
|
|
|
|
|
|
Note the subject reaching `pre_wp_mail` has already been through `wp_specialchars_decode()`. Fingerprint against the decoded form.
|
|
Note the subject reaching `pre_wp_mail` has already been through `wp_specialchars_decode()`. Fingerprint against the decoded form.
|
|
|
|
|
|
|
|
|
|
+> **A fingerprint *mismatch* is not the same as *no context*, and must be visible.** `consume()` must return three outcomes, not two:
|
|
|
|
|
+>
|
|
|
|
|
+> - **none** — no WooCommerce email was in flight. Pass through silently; this is every password reset and every non-WC `wp_mail()`.
|
|
|
|
|
+> - **mismatch** — a WC email *was* in flight, but its `to`/`subject` no longer match what `wp_mail()` received. The values were captured at `woocommerce_mail_callback_params`, *before* `apply_filters('wp_mail', …)` ran — so a plugin that rewrites the recipient or subject in a `wp_mail` filter (staging-mode redirectors, BCC archivers, some SMTP plugins) makes **every** handled mail mismatch, fall through, and send unthrottled. Collapsing this into "none" makes the plugin look installed and do nothing, with no log line to explain it. Log a **warning** naming the email id and which field diverged.
|
|
|
|
|
+> - **ok** — matched; queue it.
|
|
|
|
|
+>
|
|
|
|
|
+> The mismatch case is exactly the "silently defeated" failure the readme warns about; detection is what makes it diagnosable rather than a mystery MTA log.
|
|
|
|
|
+
|
|
|
|
|
+**Clear the was-queued flag on disabled/skipped emails too.** `woocommerce_email_disabled` (`class-wc-email.php:1150`) and `woocommerce_email_skipped` (`:1169`) fire from `send_notification()` **before** `send()`, so the `woocommerce_mail_callback` probe never runs for them and never resets the flag. `EmailLogger::log_non_send_outcome()` also consults `woocommerce_email_log_enabled` on those paths (`EmailLogger.php:246`). So two same-id emails in one request — the first queued, the second skipped for want of a billing address — would leave the flag set with a matching id, and we would swallow WooCommerce's "not sent: no recipient" log line for the second. Hook both actions at **priority 1** (before EmailLogger's 10) and clear only the queued flag there, not the whole context.
|
|
|
|
|
+
|
|
|
**Known incompatibility.** A plugin that replaces the `woocommerce_mail_callback` callable entirely bypasses `wp_mail()`, and therefore bypasses this plugin — mail sends synchronously and unthrottled. Nothing we can do at this layer. Document it.
|
|
**Known incompatibility.** A plugin that replaces the `woocommerce_mail_callback` callable entirely bypasses `wp_mail()`, and therefore bypasses this plugin — mail sends synchronously and unthrottled. Nothing we can do at this layer. Document it.
|
|
|
|
|
|
|
|
|
|
+**`pre_wp_mail` is a filter chain, not an event. Honour the value you are handed.**
|
|
|
|
|
+
|
|
|
|
|
+```php
|
|
|
|
|
+public static function maybe_enqueue( $short_circuit, $atts ) {
|
|
|
|
|
+ $ctx = Context::consume( $atts['to'], $atts['subject'] ); // ALWAYS first
|
|
|
|
|
+ if ( null !== $short_circuit ) {
|
|
|
|
|
+ return $short_circuit; // someone already sent (true) or already failed (false)
|
|
|
|
|
+ }
|
|
|
|
|
+ …
|
|
|
|
|
+}
|
|
|
|
|
+```
|
|
|
|
|
+
|
|
|
|
|
+WordPress runs `pre_wp_mail` callbacks in priority order and threads each return value into the next. A transactional-API mailer hooked below priority 10 returns `true` after it has delivered the message. Ignoring that and queueing anyway sends the mail **twice** — once immediately by them, once at our slot. Ignoring a `false` converts their failure into a reported success.
|
|
|
|
|
+
|
|
|
|
|
+The `consume()` must still run first, or an early short-circuit leaves our context set and the next unrelated `wp_mail()` in the request inherits it — the stale-context bug this section exists to prevent.
|
|
|
|
|
+
|
|
|
|
|
+**`wp_mail()` is pluggable.** A mailer that *redefines* the function never applies `pre_wp_mail` at all, and this plugin becomes silently, completely inert — no error, no log line, mail leaving unthrottled. On a host that rate-limits outbound mail an SMTP plugin is likely, not hypothetical. Detect it from `admin_init` and warn:
|
|
|
|
|
+
|
|
|
|
|
+```php
|
|
|
|
|
+$defined_in = ( new ReflectionFunction( 'wp_mail' ) )->getFileName();
|
|
|
|
|
+$is_core = wp_normalize_path( $defined_in ) === wp_normalize_path( ABSPATH . WPINC . '/pluggable.php' );
|
|
|
|
|
+```
|
|
|
|
|
+
|
|
|
### 3.1b Emails that have no context, by design
|
|
### 3.1b Emails that have no context, by design
|
|
|
|
|
|
|
|
Three WooCommerce emails call `wp_mail()` **directly**, never routing through `WC_Email::send()`, so they carry no context and always pass straight through:
|
|
Three WooCommerce emails call `wp_mail()` **directly**, never routing through `WC_Email::send()`, so they carry no context and always pass straight through:
|
|
@@ -231,18 +264,38 @@ plus a defensive clear on `shutdown`. Exactly one `wp_mail()` runs per `WC_Email
|
|
|
|
|
|
|
|
**Suppress `woocommerce_email_log_enabled` too, on the same predicate.** An earlier draft called the early WC log line "harmless". It is not: `EmailLogger` writes `status => 'sent'` to the `transactional-emails` log at enqueue, and if the deferred send later fails permanently, **nothing ever reconciles it**. The log then asserts a successful delivery that never happened — which is precisely the log an operator reaches for when a customer says the mail never arrived. A queue that lies about delivery is worse than no log.
|
|
**Suppress `woocommerce_email_log_enabled` too, on the same predicate.** An earlier draft called the early WC log line "harmless". It is not: `EmailLogger` writes `status => 'sent'` to the `transactional-emails` log at enqueue, and if the deferred send later fails permanently, **nothing ever reconciles it**. The log then asserts a successful delivery that never happened — which is precisely the log an operator reaches for when a customer says the mail never arrived. A queue that lies about delivery is worse than no log.
|
|
|
|
|
|
|
|
-Suppress both, and have the worker write both back at real send:
|
|
|
|
|
|
|
+Suppress both, and have the worker write both back at real send. **Match on the email id, not just the flag:**
|
|
|
|
|
|
|
|
```php
|
|
```php
|
|
|
-add_filter( 'woocommerce_email_log_enabled', fn( $e ) => Context::was_queued() ? false : $e, 10, 1 );
|
|
|
|
|
-add_filter( 'woocommerce_email_log_add_order_note', fn( $e ) => Context::was_queued() ? false : $e, 10, 1 );
|
|
|
|
|
|
|
+add_filter( 'woocommerce_email_log_enabled', array( __CLASS__, 'suppress' ), 10, 2 );
|
|
|
|
|
+add_filter( 'woocommerce_email_log_add_order_note', array( __CLASS__, 'suppress' ), 10, 2 );
|
|
|
|
|
+
|
|
|
|
|
+public static function suppress( $enabled, $email_id = '' ) {
|
|
|
|
|
+ if ( ! Context::was_queued() ) { return $enabled; }
|
|
|
|
|
+ if ( (string) $email_id !== Context::queued_context_id() ) { return $enabled; }
|
|
|
|
|
+ return false;
|
|
|
|
|
+}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-`Worker::send()` then emits, on the real outcome:
|
|
|
|
|
|
|
+> The flag alone leaks. `woocommerce_email_log_enabled` is *also* applied from `EmailLogger::log_non_send_outcome()` (`EmailLogger.php:246`), which runs on `woocommerce_email_disabled` and `woocommerce_email_skipped`. Both fire from `WC_Email::send_notification()` **before** `send()` — so the `woocommerce_mail_callback` probe never runs and never resets the flag.
|
|
|
|
|
+>
|
|
|
|
|
+> On a stock shop: `pending → processing` fires the customer processing email (queued, flag set) and then the admin `new_order` email. Admins routinely disable that one. We would silently swallow WooCommerce's "not sent: email type is disabled" log line for it.
|
|
|
|
|
+>
|
|
|
|
|
+> The order-note filter is correctly scoped even without this — `maybe_add_order_note()` is only reachable from `handle_woocommerce_email_sent()`, which always ran `send()` — but scope both anyway. `mark_queued()` must therefore record the context id alongside the row id.
|
|
|
|
|
+
|
|
|
|
|
+**Inherit the privacy contract you suppressed.** `EmailLogger` never writes raw recipient addresses or raw PHPMailer error strings into the `transactional-emails` log: it maps addresses through `resolve_recipient()` (username, or `guest`) and runs `redact_emails()` over error text, because that log is readable by anyone with `manage_woocommerce`. Having taken over writing the line, the worker owes the same. The plugin's own queue table storing the real address is fine and intended; the WC log is a different surface. Redact the order note's error text too — WooCommerce does.
|
|
|
|
|
+
|
|
|
|
|
+Tag the note so it groups with WooCommerce's own: `$order->add_order_note( $note, 0, false, array( 'note_group' => 'email_notification' ) )`. Use the literal string; `OrderNoteGroup` lives in the `Internal\` namespace and is not public API.
|
|
|
|
|
|
|
|
-- an order note (§8, and see the deleted-order guard below)
|
|
|
|
|
|
|
+A single `Notifier` writes both on the real outcome:
|
|
|
|
|
+
|
|
|
|
|
+- an order note (see the deleted-order guard below)
|
|
|
- a `wc_get_logger()->info()/error()` line to source `transactional-emails`, carrying the same `email_type` / `recipient` / `status` context shape `EmailLogger` uses, so existing log tooling keeps working
|
|
- a `wc_get_logger()->info()/error()` line to source `transactional-emails`, carrying the same `email_type` / `recipient` / `status` context shape `EmailLogger` uses, so existing log tooling keeps working
|
|
|
|
|
|
|
|
|
|
+> **The notifier must be reachable from EVERY terminal state, not just the worker's happy paths.** A row reaches `sent`/`failed` from four places: worker success, worker max-attempts, worker **corrupt-payload**, and reaper **max-reclaims**. The last two are easy to miss — they are terminal states that never call `wp_mail()`. Miss either and the order carries no note and `transactional-emails` carries no line: the customer never got the email and the audit trail says nothing happened. That is the §3.1c "queue that lies about delivery" defect, inverted from a false positive into a silent hole. This is why the notifier is a standalone class both the worker and the reaper call, not a private worker method.
|
|
|
|
|
+>
|
|
|
|
|
+> **Inherit the privacy contract.** `EmailLogger` maps recipients through `resolve_recipient()` (username, or `guest`) and runs `redact_emails()` over error text before writing to `transactional-emails`, because that log is readable by anyone with `manage_woocommerce`. Having suppressed it, the notifier owes the same on both the log line and the order note's error text.
|
|
|
|
|
+
|
|
|
**Guard the order note against a deleted order.** `order_id > 0` is not the same as "the order still exists". An order trashed and emptied between enqueue and send — twenty minutes is plenty — makes `wc_get_order( $order_id )` return `false`, and `false->add_order_note()` is a fatal `Error` that kills the worker **after the mail has already gone out**. Under Action Scheduler that fatal also takes the rest of the batch with it, and the row is left in `sending` for the reaper to find and *send again*.
|
|
**Guard the order note against a deleted order.** `order_id > 0` is not the same as "the order still exists". An order trashed and emptied between enqueue and send — twenty minutes is plenty — makes `wc_get_order( $order_id )` return `false`, and `false->add_order_note()` is a fatal `Error` that kills the worker **after the mail has already gone out**. Under Action Scheduler that fatal also takes the rest of the batch with it, and the row is left in `sending` for the reaper to find and *send again*.
|
|
|
|
|
|
|
|
```php
|
|
```php
|
|
@@ -314,16 +367,23 @@ Store the directory token in an `attachment_dir` column (§5). Ship `index.php`
|
|
|
|
|
|
|
|
Delete the directory the moment the row reaches `sent`. The window during which the PII exists on disk should be the queue delay and not one second more.
|
|
Delete the directory the moment the row reaches `sent`. The window during which the PII exists on disk should be the queue delay and not one second more.
|
|
|
|
|
|
|
|
-**Ordering.** The directory token is generated before the `INSERT`, so there is no chicken-and-egg — but the *row* must still exist before we start writing files we might have to clean up. Inside the enqueue path:
|
|
|
|
|
|
|
+**Ordering: copy the attachments BEFORE the row becomes durable.** An earlier revision of this plan said to `INSERT` first and rewrite the paths in a later `UPDATE`. That is wrong, and subtly so:
|
|
|
|
|
+
|
|
|
|
|
+> Between the `INSERT` and the `UPDATE` there is a durable `pending` row whose payload points at the invoice plugin's **temp** paths. A PHP fatal in that window leaves it there permanently. `rollback()` cannot run after a fatal — which is exactly why §2.1 makes the reaper the safety net — and the reaper reschedules rows without re-validating attachments. It would faithfully resurrect a mail that can never send, burn its retry budget on `wp_mail()` failures, and bury it in `failed`.
|
|
|
|
|
|
|
|
-1. Generate `$dir_token`. `INSERT` the row with `state='pending'`, `attachment_dir = $dir_token`, and the **raw** payload (original attachment paths), obtaining `$row_id`.
|
|
|
|
|
-2. Copy attachments into `uploads/studiou-wcmq-attachments/{$dir_token}/`.
|
|
|
|
|
-3. `UPDATE` the row's payload with the rewritten paths.
|
|
|
|
|
-4. `as_schedule_single_action( $slot, …, array( (int) $row_id ), … )`.
|
|
|
|
|
|
|
+The directory token is generated up front, so there is no chicken-and-egg to justify the wrong order. Inside the enqueue path:
|
|
|
|
|
|
|
|
-If step 2, 3 or 4 fails: delete the directory, delete the row, return `false`, let the mail send synchronously (§2). Do not leave a row pointing at paths that were never copied.
|
|
|
|
|
|
|
+1. Generate `$dir_token`.
|
|
|
|
|
+2. Copy attachments into `uploads/studiou-wcmq-attachments/{$dir_token}/` and rewrite the payload's paths.
|
|
|
|
|
+3. `json_encode` the payload.
|
|
|
|
|
+4. `INSERT` the row with `state='pending'`, `attachment_dir = $dir_token`, obtaining `$row_id`.
|
|
|
|
|
+5. `as_schedule_single_action( $slot, …, array( (int) $row_id ), … )`.
|
|
|
|
|
|
|
|
-Rollback cannot run after a fatal, so the reaper (§8.1) and retention both treat "directory with no surviving row" as garbage and delete it.
|
|
|
|
|
|
|
+If 2 or 3 fails: delete the directory, return `false`, send synchronously (§2). No row exists yet, so there is nothing to roll back. If 4 fails: delete the directory. If 5 fails: delete both.
|
|
|
|
|
+
|
|
|
|
|
+The row is now **never durable with paths we do not own**, so the only fatal window left — between `INSERT` and `as_schedule_single_action()` — leaves a row the reaper can safely reschedule. The single cost is an orphaned directory when we bail before the `INSERT`; retention collects those.
|
|
|
|
|
+
|
|
|
|
|
+Rollback cannot run after a fatal, so "directory with no surviving row" must be treated as garbage and collected. **Retention owns this, not the reaper.** The daily sweep's `purge_orphan_attachment_dirs()` deletes any token directory older than one day with no matching row — the one-day floor is what keeps it from racing an in-flight enqueue whose `INSERT` has not landed yet. The reaper (5-min cadence) deliberately does not touch directories: a shorter floor would risk that race, and reusing the one-day floor would gain nothing over retention. The cost is that a directory orphaned by a fatal *between* `mkdir` and `INSERT` — a rare fatal inside a millisecond window — can hold attachment PII on disk for up to ~a day. Accepted; documented in the readme's limitations. (An earlier draft said "the reaper and retention both" collect these — that was aspirational; retention alone does.)
|
|
|
|
|
|
|
|
**Lifetime.** Delete the directory when the row reaches `sent`, and when retention sweeps it. **Do not delete it when the row reaches `failed`** — `failed` rows are manually retriable from the admin (§11, Phase 8), and a retry that fires without its attachments sends a broken email that looks fine. Attachments for `failed` rows die with the row at `retention_failed_days`.
|
|
**Lifetime.** Delete the directory when the row reaches `sent`, and when retention sweeps it. **Do not delete it when the row reaches `failed`** — `failed` rows are manually retriable from the admin (§11, Phase 8), and a retry that fires without its attachments sends a broken email that looks fine. Attachments for `failed` rows die with the row at `retention_failed_days`.
|
|
|
|
|
|
|
@@ -346,6 +406,37 @@ try {
|
|
|
|
|
|
|
|
Same discipline for the `wp_mail_failed` listener (§8) and for the `GET_LOCK` release (§4) — every one of them is a paired acquire/release across code that can throw.
|
|
Same discipline for the `wp_mail_failed` listener (§8) and for the `GET_LOCK` release (§4) — every one of them is a paired acquire/release across code that can throw.
|
|
|
|
|
|
|
|
|
|
+### 3.4a The `wp_mail` filter runs *before* `pre_wp_mail`, so the worker double-applies it
|
|
|
|
|
+
|
|
|
|
|
+`wp_mail()` opens with:
|
|
|
|
|
+
|
|
|
|
|
+```php
|
|
|
|
|
+$atts = apply_filters( 'wp_mail', compact( 'to', 'subject', 'message', 'headers', 'attachments' ) );
|
|
|
|
|
+$pre_wp_mail = apply_filters( 'pre_wp_mail', null, $atts );
|
|
|
|
|
+```
|
|
|
|
|
+
|
|
|
|
|
+The `$atts` we snapshot have therefore **already been through every `wp_mail` filter on the site.** When the worker later calls `wp_mail()` with that snapshot, they all run a second time — against their own prior output.
|
|
|
|
|
+
|
|
|
|
|
+A plugin appending an unsubscribe footer appends it twice. A BCC-archive filter adds a duplicate `Bcc:`. A staging-mode filter that rewrites the recipient rewrites the already-rewritten address. Queued mail silently differs from synchronous mail, and nobody will connect it to this plugin.
|
|
|
|
|
+
|
|
|
|
|
+`wp_mail_from` / `wp_mail_from_name` / `wp_mail_content_type` need no such treatment — they are idempotent, and §3.2 has already baked the resolved sender into the headers.
|
|
|
|
|
+
|
|
|
|
|
+Detach the whole hook for the duration of the worker's send, and restore it in `finally`:
|
|
|
|
|
+
|
|
|
|
|
+```php
|
|
|
|
|
+global $wp_filter;
|
|
|
|
|
+$saved = $wp_filter['wp_mail'] ?? null;
|
|
|
|
|
+if ( null !== $saved ) { unset( $wp_filter['wp_mail'] ); }
|
|
|
|
|
+try {
|
|
|
|
|
+ return (bool) wp_mail( ... );
|
|
|
|
|
+} finally {
|
|
|
|
|
+ Interceptor::$sending = false;
|
|
|
|
|
+ if ( null !== $saved ) { $wp_filter['wp_mail'] = $saved; }
|
|
|
|
|
+}
|
|
|
|
|
+```
|
|
|
|
|
+
|
|
|
|
|
+`remove_all_filters('wp_mail')` is **not** an option — it would strip the callbacks for the rest of the request, and under Action Scheduler that means the rest of the batch. Restore, always.
|
|
|
|
|
+
|
|
|
### 3.5 Consequence: the queued body is a point-in-time snapshot
|
|
### 3.5 Consequence: the queued body is a point-in-time snapshot
|
|
|
|
|
|
|
|
Because we store the rendered HTML rather than an order ID, an order edited between enqueue and send will produce an email showing the **old** data. This is a deliberate trade of the trigger-detach approach's always-fresh rendering.
|
|
Because we store the rendered HTML rather than an order ID, an order edited between enqueue and send will produce an email showing the **old** data. This is a deliberate trade of the trigger-detach approach's always-fresh rendering.
|
|
@@ -647,20 +738,28 @@ Worker::send( $row_id )
|
|
|
replace it with a SELECT-then-UPDATE.
|
|
replace it with a SELECT-then-UPDATE.
|
|
|
NOTE: attempts is NOT incremented here. See §5.
|
|
NOTE: attempts is NOT incremented here. See §5.
|
|
|
2. Load row. $payload = json_decode( $row->payload, true );
|
|
2. Load row. $payload = json_decode( $row->payload, true );
|
|
|
- if ( ! is_array( $payload ) || JSON_ERROR_NONE !== json_last_error() )
|
|
|
|
|
- → state='failed', last_error='corrupt payload', claimed_at=NULL, return.
|
|
|
|
|
|
|
+ if corrupt → update_row_if_state('failed', last_error='corrupt payload',
|
|
|
|
|
+ claimed_at=NULL; WHERE state='sending');
|
|
|
|
|
+ if 1 row changed → Notifier::annotate(row, false, 'corrupt payload');
|
|
|
|
|
+ return. // terminal state — the order MUST get a note (§3.1c)
|
|
|
3. $this->last_error = null; // reset buffer BEFORE the send
|
|
3. $this->last_error = null; // reset buffer BEFORE the send
|
|
|
4. Interceptor::$sending = true;
|
|
4. Interceptor::$sending = true;
|
|
|
try { $ok = wp_mail( ... ); }
|
|
try { $ok = wp_mail( ... ); }
|
|
|
finally { Interceptor::$sending = false; }
|
|
finally { Interceptor::$sending = false; }
|
|
|
$error = $this->last_error; $this->last_error = null; // consume
|
|
$error = $this->last_error; $this->last_error = null; // consume
|
|
|
- UPDATE ... SET attempts = attempts + 1 WHERE id = %d // AFTER the attempt
|
|
|
|
|
- 5. Success → state='sent', sent_at=now, claimed_at=NULL ← commit FIRST
|
|
|
|
|
- then, in its own try/catch: delete attachment dir,
|
|
|
|
|
- write the order note + WC log line (§3.1c).
|
|
|
|
|
- Nothing here may throw back into the send path.
|
|
|
|
|
|
|
+ $attempts = (int) $row->attempts + 1; // computed, not written yet
|
|
|
|
|
+ 5. Success → ONE state-guarded UPDATE: state='sent', sent_at=now, claimed_at=NULL,
|
|
|
|
|
+ last_error=NULL, attempts=$attempts; WHERE state='sending'
|
|
|
|
|
+ ← check affected === 1. If not, the mail HAS gone out but the row
|
|
|
|
|
+ was deleted/reclaimed or the DB errored: log loudly, do NOT delete
|
|
|
|
|
+ the attachment dir, do NOT write a success note. The reaper may
|
|
|
|
|
+ resend it — keep it sendable.
|
|
|
|
|
+ then: delete attachment dir, Notifier::annotate(row, true, null).
|
|
|
|
|
+ Notifier never throws (it try/catches internally).
|
|
|
Failure → if attempts >= max_attempts:
|
|
Failure → if attempts >= max_attempts:
|
|
|
- state='failed', claimed_at=NULL, KEEP attachment dir (§3.3)
|
|
|
|
|
|
|
+ update_row_if_state('failed', claimed_at=NULL, attempts=$attempts;
|
|
|
|
|
+ WHERE state='sending'); KEEP attachment dir (§3.3);
|
|
|
|
|
+ if 1 row changed → Notifier::annotate(row, false, $error);
|
|
|
else:
|
|
else:
|
|
|
[ $slot, ] = Queue::with_slot_lock( now + backoff,
|
|
[ $slot, ] = Queue::with_slot_lock( now + backoff,
|
|
|
fn( $s ) => /* UPDATE state='pending', claimed_at=NULL,
|
|
fn( $s ) => /* UPDATE state='pending', claimed_at=NULL,
|
|
@@ -673,6 +772,10 @@ Worker::send( $row_id )
|
|
|
6. If state is 'draining' and no pending/sending rows remain → 'disabled'.
|
|
6. If state is 'draining' and no pending/sending rows remain → 'disabled'.
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
|
|
+**Every terminal write is state-guarded on `sending`.** Use `UPDATE … WHERE id = %d AND state = 'sending'`, not a bare `WHERE id`. Between the claim and the commit the row can be deleted from the admin or reclaimed by a parallel reaper; a bare update reports success anyway (`$wpdb->update()` returns 0 changed rows as distinct from `false`, and a plain "did it error?" check treats 0 as fine). The worker would then delete attachments and write an order note for a row that no longer exists. Provide `DB::update_row_if_state()` and check for exactly one affected row before any post-commit bookkeeping. The same guard closes the reaper's infinite-loop bug — a re-slot whose `UPDATE` changed nothing must **not** schedule an action, or the row stays `sending` with unpersisted `reclaims`, and every sweep reschedules a fresh dead action forever without ever tripping `max_reclaims`.
|
|
|
|
|
+
|
|
|
|
|
+**`attempts` is folded into the commit, never written separately.** A standalone `UPDATE … SET attempts = attempts + 1` after `wp_mail()` returns but before the state commit widens exactly the window that "commit first, bookkeep second" exists to close: a fatal between the two leaves a row that has been *attempted* but not *resolved*. Because the worker holds the claim, nobody else writes that column, so computing `$attempts = $row->attempts + 1` in PHP and setting it explicitly in the one terminal `UPDATE` is safe and removes the window entirely.
|
|
|
|
|
+
|
|
|
**Decode defensively.** §2 guards `json_encode()` returning `false`; the read side must be symmetric. `json_decode()` returns `null` on a truncated or corrupted `LONGTEXT` — a MySQL `max_allowed_packet` truncation, a botched migration, a partially-written `UPDATE` from a crashed enqueue. Feeding `null` into `wp_mail( $payload['to'], … )` throws on array access. Fail the row loudly instead; a corrupt payload is not retriable, so do not burn attempts on it.
|
|
**Decode defensively.** §2 guards `json_encode()` returning `false`; the read side must be symmetric. `json_decode()` returns `null` on a truncated or corrupted `LONGTEXT` — a MySQL `max_allowed_packet` truncation, a botched migration, a partially-written `UPDATE` from a crashed enqueue. Feeding `null` into `wp_mail( $payload['to'], … )` throws on array access. Fail the row loudly instead; a corrupt payload is not retriable, so do not burn attempts on it.
|
|
|
|
|
|
|
|
**`$row_id` arrives from Action Scheduler as an int** (it round-trips through `json_decode` of the action's args), so `Worker::send( $row_id )` is safe. Rows read back from `$wpdb` are **numeric strings** — cast before handing them to any Action Scheduler function. See §8.1.
|
|
**`$row_id` arrives from Action Scheduler as an int** (it round-trips through `json_decode` of the action's args), so `Worker::send( $row_id )` is safe. Rows read back from `$wpdb` are **numeric strings** — cast before handing them to any Action Scheduler function. See §8.1.
|
|
@@ -726,6 +829,12 @@ if ( ! as_has_scheduled_action( 'studiou_wcmq_send', array( $row_id ), 'studiou-
|
|
|
|
|
|
|
|
> **The cast is load-bearing.** Action Scheduler matches actions by hashing `json_encode( $args )`. The action was scheduled with `array( (int) $row_id )` → `"[123]"`. A `$wpdb` result gives `array( "123" )` → `"[\"123\"]"`. The hashes differ, `as_has_scheduled_action()` returns `false` for a perfectly healthy row, and the reaper duplicates its action — two workers race the same row on every sweep. Cast every id that crosses into an Action Scheduler call.
|
|
> **The cast is load-bearing.** Action Scheduler matches actions by hashing `json_encode( $args )`. The action was scheduled with `array( (int) $row_id )` → `"[123]"`. A `$wpdb` result gives `array( "123" )` → `"[\"123\"]"`. The hashes differ, `as_has_scheduled_action()` returns `false` for a perfectly healthy row, and the reaper duplicates its action — two workers race the same row on every sweep. Cast every id that crosses into an Action Scheduler call.
|
|
|
|
|
|
|
|
|
|
+> **(a) and (b) are different faults and must not share a counter.**
|
|
|
|
|
+>
|
|
|
|
|
+> Only (a) is evidence that a worker died holding this row. (b) is a row that was **never claimed and never attempted** — its Action Scheduler action simply went missing. Charging (b) to `reclaims` means a queue that repeatedly loses actions (a pruned AS table, a failed retry re-schedule) eventually marks perfectly deliverable mail `failed` with `last_error = 'worker repeatedly crashed'` — a diagnosis that is both false and unactionable.
|
|
|
|
|
+>
|
|
|
|
|
+> `recover_crashed()` bumps `reclaims` and honours `max_reclaims`. `repair_orphan()` bumps nothing and simply re-slots. There is no runaway risk: the re-slot pushes `scheduled_at` into the future, so the next sweep will not see the row until the grace window elapses again.
|
|
|
|
|
+
|
|
|
**Every recovered row is re-slotted, not merely un-claimed.** A row reclaimed with its original `scheduled_at` now lies in the *past*, so Action Scheduler fires it on the very next pass — and if the reaper recovers thirty rows at once, all thirty go out simultaneously. That is the burst this plugin exists to prevent, delivered by its own recovery path. Re-slot through the same locked allocator, and **guard the schedule call exactly as enqueue and retry do**:
|
|
**Every recovered row is re-slotted, not merely un-claimed.** A row reclaimed with its original `scheduled_at` now lies in the *past*, so Action Scheduler fires it on the very next pass — and if the reaper recovers thirty rows at once, all thirty go out simultaneously. That is the burst this plugin exists to prevent, delivered by its own recovery path. Re-slot through the same locked allocator, and **guard the schedule call exactly as enqueue and retry do**:
|
|
|
|
|
|
|
|
```php
|
|
```php
|
|
@@ -811,34 +920,44 @@ studiou-wc-mail-queue/
|
|
|
studiou-wc-mail-queue.php mu-plugins/ stub (see §9)
|
|
studiou-wc-mail-queue.php mu-plugins/ stub (see §9)
|
|
|
includes/
|
|
includes/
|
|
|
utils-log.php UtilsLog — adapted, gated on the debug setting
|
|
utils-log.php UtilsLog — adapted, gated on the debug setting
|
|
|
- class-wcmq-db.php schema, maybe_upgrade_db, queue+log CRUD
|
|
|
|
|
- class-wcmq-settings.php option get/set, defaults, sanitization
|
|
|
|
|
- class-wcmq-context.php woocommerce_mail_callback probe, consume-once
|
|
|
|
|
- class-wcmq-interceptor.php pre_wp_mail, payload snapshot, re-entrancy guard
|
|
|
|
|
- class-wcmq-queue.php slot ladder, GET_LOCK, enqueue, AS scheduling
|
|
|
|
|
- class-wcmq-worker.php studiou_wcmq_send handler, claim, retry, backoff
|
|
|
|
|
- class-wcmq-retention.php daily sweep of queue rows, log rows, attachment dirs
|
|
|
|
|
|
|
+ class-wcmq-db.php schema, maybe_upgrade_db, queue+log CRUD, update_row_if_state
|
|
|
|
|
+ class-wcmq-settings.php option get/set, defaults, sanitization, RUNTIME_ALIAS_IDS
|
|
|
class-wcmq-state.php enabled/draining/disabled machine
|
|
class-wcmq-state.php enabled/draining/disabled machine
|
|
|
- class-wcmq-admin.php menu, tabs, form handlers, AJAX
|
|
|
|
|
- class-wcmq-queue-list-table.php WP_List_Table — queue
|
|
|
|
|
- class-wcmq-log-list-table.php WP_List_Table — log
|
|
|
|
|
|
|
+ class-wcmq-context.php woocommerce_mail_callback probe, consume (none/mismatch/ok)
|
|
|
|
|
+ class-wcmq-queue.php allocate_slot, with_slot_lock, enqueue, reschedule, attachments
|
|
|
|
|
+ class-wcmq-interceptor.php pre_wp_mail, payload snapshot, EmailLogger suppression
|
|
|
|
|
+ class-wcmq-notifier.php terminal-outcome order note + WC log line (see §3.1c)
|
|
|
|
|
+ class-wcmq-worker.php studiou_wcmq_send handler, claim, dispatch, retry, backoff
|
|
|
|
|
+ class-wcmq-reaper.php studiou_wcmq_reap — rescues stranded rows (own class)
|
|
|
|
|
+ class-wcmq-retention.php daily sweep of queue rows, log rows, attachment dirs
|
|
|
|
|
+ class-wcmq-admin.php menu, tabs, admin-post form handlers
|
|
|
views/
|
|
views/
|
|
|
- admin-page.php tab shell
|
|
|
|
|
|
|
+ admin-page.php tab shell (state banner + counts + tab bar)
|
|
|
tab-settings.php
|
|
tab-settings.php
|
|
|
- tab-queue.php
|
|
|
|
|
- tab-log.php
|
|
|
|
|
|
|
+ tab-queue.php plain wp-list-table markup, bulk retry/delete
|
|
|
|
|
+ tab-log.php plain wp-list-table markup, level filter, clear
|
|
|
assets/css/admin.css
|
|
assets/css/admin.css
|
|
|
assets/js/admin.js
|
|
assets/js/admin.js
|
|
|
languages/
|
|
languages/
|
|
|
studiou-wc-mail-queue.pot
|
|
studiou-wc-mail-queue.pot
|
|
|
studiou-wc-mail-queue-cs_CZ.po
|
|
studiou-wc-mail-queue-cs_CZ.po
|
|
|
- studiou-wc-mail-queue-cs_CZ.mo
|
|
|
|
|
|
|
+ studiou-wc-mail-queue-cs_CZ.mo (hand-compiled; not present in this repo)
|
|
|
|
|
+ loader/studiou-wc-mail-queue.php mu-plugins/ stub
|
|
|
|
|
+ .gitignore excludes the vendored WC tree
|
|
|
docs/
|
|
docs/
|
|
|
analyza-woocommerce-email-throttling.md
|
|
analyza-woocommerce-email-throttling.md
|
|
|
sample-wc-mail-throttle.php
|
|
sample-wc-mail-throttle.php
|
|
|
|
|
+ Wiki/woocommerce-emailing.md distilled WC reference
|
|
|
plans/implement-plan-00.md (this file)
|
|
plans/implement-plan-00.md (this file)
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
|
|
+**As-built deviations from the layout above, all deliberate:**
|
|
|
|
|
+
|
|
|
|
|
+- **`class-wcmq-notifier.php` is new** and not in the original plan. It exists because a terminal-outcome note must be written from four call sites (§3.1c); a private worker method could not serve the reaper's max-reclaims path.
|
|
|
|
|
+- **`class-wcmq-reaper.php` is split out** of retention. The reaper has three independent triggers (per-send, recurring, admin_init) and does not belong inside the daily sweep.
|
|
|
|
|
+- **No `class-wcmq-*-list-table.php`.** The queue and log views render `<table class="wp-list-table">` inline. Less machinery, no untestable `WP_List_Table` subclass.
|
|
|
|
|
+- **No AJAX handlers.** The admin posts to `admin-post.php`; handlers still verify nonce + `manage_woocommerce`. This sidesteps output-buffer and JSON-envelope plumbing entirely. Phase 8's acceptance criteria (nonce, cap, correct counts) still hold.
|
|
|
|
|
+
|
|
|
Naming follows the repo: constants `STUDIOU_WCMQ_*`, CSS prefix `studiou-wcmq-`, JS namespace `studiouWcmq`, nonce `studiou-wcmq-nonce`, error-log prefix `STUDIOU WC MAIL:`, text domain `studiou-wc-mail-queue`, AS group `studiou-wcmq`, AS hook `studiou_wcmq_send`.
|
|
Naming follows the repo: constants `STUDIOU_WCMQ_*`, CSS prefix `studiou-wcmq-`, JS namespace `studiouWcmq`, nonce `studiou-wcmq-nonce`, error-log prefix `STUDIOU WC MAIL:`, text domain `studiou-wc-mail-queue`, AS group `studiou-wcmq`, AS hook `studiou_wcmq_send`.
|
|
|
|
|
|
|
|
Managers register their own hooks in their constructors. The main class only guards on WooCommerce, `require_once`s the includes, and instantiates. Nothing else.
|
|
Managers register their own hooks in their constructors. The main class only guards on WooCommerce, `require_once`s the includes, and instantiates. Nothing else.
|
|
@@ -869,7 +988,7 @@ Main file with the plugin header, `defined('ABSPATH') || exit;`, the five consta
|
|
|
|
|
|
|
|
`class-wcmq-context.php` (probe, consume-once, `to`+`subject` fingerprint per §3.1) and `class-wcmq-interceptor.php` (`pre_wp_mail`, payload snapshot including resolved `From:` per §3.2, attachment copying per §3.3, re-entrancy guard per §3.4). Do **not** wire the queue yet — log the decision and return `null` so everything still sends normally.
|
|
`class-wcmq-context.php` (probe, consume-once, `to`+`subject` fingerprint per §3.1) and `class-wcmq-interceptor.php` (`pre_wp_mail`, payload snapshot including resolved `From:` per §3.2, attachment copying per §3.3, re-entrancy guard per §3.4). Do **not** wire the queue yet — log the decision and return `null` so everything still sends normally.
|
|
|
|
|
|
|
|
-*Accepts when:* with `debug` on, completing a single order writes one log line naming `customer_completed_order`; a **WooCommerce** password reset writes `customer_reset_password`; a **wp-admin** password reset and a low-stock alert both write an empty context; all of them still arrive normally. **No mail is queued or lost in this phase.** Verify the captured `From:` matches WooCommerce → Settings → Emails, not `wordpress@`. Verify the fingerprint rejects a stale context by temporarily setting one by hand and firing an unrelated `wp_mail()`.
|
|
|
|
|
|
|
+*Accepts when:* with `debug` on, completing a single order writes one log line naming `customer_completed_order` — the line comes *before* any queue/pass-through decision, so it must appear even in this phase where nothing is queued; a **WooCommerce** password reset writes `customer_reset_password`; a **wp-admin** password reset and a low-stock alert produce no such line (status `none`, passed through silently). All still arrive normally. **No mail is queued or lost in this phase.** Verify the captured `From:` matches WooCommerce → Settings → Emails, not `wordpress@`. Verify the fingerprint rejects a stale context by temporarily setting one by hand and firing an unrelated `wp_mail()`. Add a `wp_mail` filter that rewrites the subject, complete an order, and confirm a **warning** log line naming the divergence — not silence.
|
|
|
|
|
|
|
|
### Phase 4 — Queue and slot allocation
|
|
### Phase 4 — Queue and slot allocation
|
|
|
`class-wcmq-queue.php`: `allocate_slot($earliest)` per §4.2, `with_slot_lock()` **returning `[$slot, $result]`** per §4.3, attachment copy ordering per §3.3, then `as_schedule_single_action($slot, 'studiou_wcmq_send', array((int) $row_id), 'studiou-wcmq')` — note the `(int)` cast (§8.1). Flip the interceptor to return `true` — **but only on a successful enqueue** (§2).
|
|
`class-wcmq-queue.php`: `allocate_slot($earliest)` per §4.2, `with_slot_lock()` **returning `[$slot, $result]`** per §4.3, attachment copy ordering per §3.3, then `as_schedule_single_action($slot, 'studiou_wcmq_send', array((int) $row_id), 'studiou-wcmq')` — note the `(int)` cast (§8.1). Flip the interceptor to return `true` — **but only on a successful enqueue** (§2).
|
|
@@ -894,9 +1013,12 @@ Specifically:
|
|
|
- A retry lands on a slot that respects the rate cap, **not** at a bare `now + backoff` — assert `scheduled_at` is at least `interval` from every other pending row (§4.2, §4.3).
|
|
- A retry lands on a slot that respects the rate cap, **not** at a bare `now + backoff` — assert `scheduled_at` is at least `interval` from every other pending row (§4.2, §4.3).
|
|
|
- Firing `studiou_wcmq_send` twice for the same row sends exactly once.
|
|
- Firing `studiou_wcmq_send` twice for the same row sends exactly once.
|
|
|
- `kill -9` mid-send leaves a `sending` row that the reaper returns to `pending` at a **future** slot after `claim_timeout`, bumping `reclaims` and **not** `attempts` (§5, §8.1).
|
|
- `kill -9` mid-send leaves a `sending` row that the reaper returns to `pending` at a **future** slot after `claim_timeout`, bumping `reclaims` and **not** `attempts` (§5, §8.1).
|
|
|
-- A row corrupted to invalid JSON fails immediately with `last_error = 'corrupt payload'` and does **not** consume an attempt.
|
|
|
|
|
|
|
+- A row corrupted to invalid JSON fails immediately with `last_error = 'corrupt payload'`, does **not** consume an attempt, **and still writes a failure order note** — a terminal state must never leave the order silent (§3.1c).
|
|
|
|
|
+- A row driven past `max_reclaims` by repeated crashes lands in `failed` **with a failure order note** — the reaper's terminal path, easy to miss.
|
|
|
|
|
+- Every terminal state — `sent`, `failed` via max-attempts, `failed` via corrupt-payload, `failed` via max-reclaims — leaves exactly one note and one `transactional-emails` log line; the log line carries a **username or `guest`**, never a raw address, and any error text has addresses redacted.
|
|
|
- A partial refund queues as `customer_partially_refunded_order` — which requires the alias from §6, and will fail if the selector was built from `get_emails()` alone.
|
|
- A partial refund queues as `customer_partially_refunded_order` — which requires the alias from §6, and will fail if the selector was built from `get_emails()` alone.
|
|
|
-- Delete an order between enqueue and send: the mail still goes out, the row still reaches `sent`, and the worker does **not** fatal (§3.1c).
|
|
|
|
|
|
|
+- Delete an order between enqueue and send: the mail still goes out, the row reaches `sent` **or the commit reports 0 rows and the attachment dir is left intact for the reaper**, and the worker does **not** fatal (§3.1c).
|
|
|
|
|
+- Delete a row from the admin while a worker holds it: the worker's terminal `UPDATE` changes 0 rows, so it does **not** delete attachments or write a note for the vanished row.
|
|
|
|
|
|
|
|
### Phase 6 — State machine
|
|
### Phase 6 — State machine
|
|
|
`class-wcmq-state.php`. Enable/disable/drain per §7.
|
|
`class-wcmq-state.php`. Enable/disable/drain per §7.
|
|
@@ -949,7 +1071,16 @@ Pass `$unique = true` *and* pre-check with `as_next_scheduled_action()` — the
|
|
|
`class-wcmq-admin.php`, two `WP_List_Table`s, three tab views. Submenu under **WooCommerce**, capability `manage_woocommerce`, following `Import_Manager` in `studiou-wc-ord-print-statuses`.
|
|
`class-wcmq-admin.php`, two `WP_List_Table`s, three tab views. Submenu under **WooCommerce**, capability `manage_woocommerce`, following `Import_Manager` in `studiou-wc-ord-print-statuses`.
|
|
|
|
|
|
|
|
- **Settings** — rate, retention, debug toggle, handled-contexts checkboxes rendered from `WC()->mailer()->get_emails()`, enable/disable control.
|
|
- **Settings** — rate, retention, debug toggle, handled-contexts checkboxes rendered from `WC()->mailer()->get_emails()`, enable/disable control.
|
|
|
-- **Queue** — counts by state, list table (recipient, context, order, state, attempts, scheduled, sent), per-row Retry and Delete, bulk Retry/Delete, "Send next now".
|
|
|
|
|
|
|
+- **Queue** — counts by state, list table (recipient, context, order, state, attempts, scheduled, sent), per-row Retry and Delete, bulk Retry/Delete.
|
|
|
|
|
+
|
|
|
|
|
+ **Retry is a destructive action and needs a state guard, not just a confirm dialog.**
|
|
|
|
|
+
|
|
|
|
|
+ - **Never retry a `sent` row.** Its attachment directory was deleted on send, so the resend arrives with missing files *and* the customer receives the email twice. Select-all on the "All" or "Sent" filter must be harmless. This is one misclick from mailing the whole customer list again.
|
|
|
|
|
+ - **Never touch a row a worker is actively holding** — `sending` with `claimed_at` newer than `claim_timeout` — for *either* action. Retrying it delivers twice; deleting it pulls the attachment directory out from under a live `wp_mail()`. Past the claim timeout the worker is presumed dead (same rule as §8.1) and the row is fair game.
|
|
|
|
|
+ - **Unschedule before re-slotting.** A `pending` row already owns an Action Scheduler action; `reschedule()` without `as_unschedule_all_actions()` leaves two actions for one row.
|
|
|
|
|
+ - **Reset `reclaims` as well as `attempts`.** A manual retry is a fresh start; inheriting an accumulated crash budget means one more crash fails the row.
|
|
|
|
|
+
|
|
|
|
|
+ Order links must come from `WC_Order::get_edit_order_url()`, never a hardcoded `page=wc-orders` URL — that admin page does not exist when order storage is legacy posts. Guard on the order still existing.
|
|
|
- **Log** — filterable by level, Clear Log behind a confirmation dialog.
|
|
- **Log** — filterable by level, Clear Log behind a confirmation dialog.
|
|
|
|
|
|
|
|
AJAX handlers: verify nonce, check `manage_woocommerce`, clean the output buffer, try/catch, reply via `wp_send_json_success()` / `wp_send_json_error()`. Sanitize in, escape out.
|
|
AJAX handlers: verify nonce, check `manage_woocommerce`, clean the output buffer, try/catch, reply via `wp_send_json_success()` / `wp_send_json_error()`. Sanitize in, escape out.
|
|
@@ -1041,6 +1172,9 @@ Also verify: WordPress password resets, low-stock alerts, and other non-handled
|
|
|
- **`TRUNCATE` fails on restricted hosting.** It needs the `DROP` privilege, which the target shared hosts often withhold. Clear Log uses a batched `DELETE` (§5).
|
|
- **`TRUNCATE` fails on restricted hosting.** It needs the `DROP` privilege, which the target shared hosts often withhold. Clear Log uses a batched `DELETE` (§5).
|
|
|
- **`order_id` never populated.** `pre_wp_mail` cannot see the order; only `$email->object` in the `woocommerce_mail_callback` probe can (§3.1). Miss it and the admin Order column, the replacement order note, and any per-order debugging are all dead — while the schema looks correct.
|
|
- **`order_id` never populated.** `pre_wp_mail` cannot see the order; only `$email->object` in the `woocommerce_mail_callback` probe can (§3.1). Miss it and the admin Order column, the replacement order note, and any per-order debugging are all dead — while the schema looks correct.
|
|
|
- **Order audit trail deleted rather than deferred.** Suppressing `EmailLogger`'s note by *email type* rather than by *was-this-mail-actually-queued* strips the note from handled-type mail that sent synchronously (draining, disabled, AS missing, enqueue failed), with no worker to write a replacement (§3.1c).
|
|
- **Order audit trail deleted rather than deferred.** Suppressing `EmailLogger`'s note by *email type* rather than by *was-this-mail-actually-queued* strips the note from handled-type mail that sent synchronously (draining, disabled, AS missing, enqueue failed), with no worker to write a replacement (§3.1c).
|
|
|
|
|
+- **A terminal state that writes no note.** The corrupt-payload and max-reclaims paths reach `failed` without ever calling `wp_mail()`, so it is easy to leave them without a `Notifier::annotate()`. The order then records nothing about a mail the customer never received — the audit-trail hole inverted from the previous risk. The notifier is a shared class specifically so all four terminal paths reach it (§3.1c).
|
|
|
|
|
+- **The reaper looping forever.** A re-slot whose state-guarded `UPDATE` changes no row must not schedule an action; otherwise the row stays `sending`, `reclaims` never persists, and every 5-minute sweep schedules a fresh dead action without tripping `max_reclaims` (§8.1). Low probability (needs a DB error or a concurrent mutation) but unbounded if it hits.
|
|
|
|
|
+- **Fingerprint mismatch mistaken for "no email in flight".** A `wp_mail` filter that rewrites recipient/subject makes every handled mail fall through unthrottled and silent. `consume()` distinguishes `none` from `mismatch` and logs a warning on the latter (§3.1), so an inert plugin is diagnosable instead of a mystery.
|
|
|
- **Rate set above the host's real limit.** The plugin cannot discover the limit. Ask the host's support for the exact outbound messages-per-minute figure and set `rate_per_minute` below it. Put this in the readme; a misconfigured cap reproduces the original bug with extra steps.
|
|
- **Rate set above the host's real limit.** The plugin cannot discover the limit. Ask the host's support for the exact outbound messages-per-minute figure and set `rate_per_minute` below it. Put this in the readme; a misconfigured cap reproduces the original bug with extra steps.
|
|
|
|
|
|
|
|
---
|
|
---
|