|
| 1 | +# ADR 0001 — Handle Mailchimp errors at the dispatch boundary |
| 2 | + |
| 3 | +## Status |
| 4 | + |
| 5 | +Accepted (2026-07-17) |
| 6 | + |
| 7 | +## Context |
| 8 | + |
| 9 | +The plugin synchronizes Sylius data (carts, orders, products, stores, customers/members) |
| 10 | +to Mailchimp through Symfony Messenger: event subscribers react to storefront events |
| 11 | +(add-to-cart, checkout, profile update, ...), call an *enqueuer* which dispatches a |
| 12 | +message, and a *message handler* performs the actual Mailchimp API call. |
| 13 | + |
| 14 | +Two facts create tension: |
| 15 | + |
| 16 | +1. **The plugin ships no Messenger routing configuration.** With a default Symfony setup |
| 17 | + every message is handled **synchronously, inside the same HTTP request** that |
| 18 | + dispatched it. Any exception thrown by a handler therefore propagates into the |
| 19 | + storefront request. This was observed in production: a Mailchimp 400 during a cart |
| 20 | + sync turned an add-to-cart action into a 500 error page. A Mailchimp sync is a |
| 21 | + non-critical marketing side effect and must never break the shopping flow. |
| 22 | + |
| 23 | +2. **This is a public plugin.** Host applications will route these messages to an |
| 24 | + asynchronous transport and rely on Messenger's standard error machinery: automatic |
| 25 | + retries with backoff and the failure transport. That machinery only works if the |
| 26 | + handler **throws** on failure. A handler that catches everything reports "handled" to |
| 27 | + Messenger even when the sync failed, silently disabling retries and the failure queue |
| 28 | + for every async host, with no way to opt out. |
| 29 | + |
| 30 | +A first implementation used catch-all blocks inside every handler ("never throw"). |
| 31 | +It solved problem 1 but caused problem 2, so it was rejected and reworked into the |
| 32 | +decision below. |
| 33 | + |
| 34 | +### Options considered |
| 35 | + |
| 36 | +- **A. Catch everything in the handlers ("swallow always").** Simple, storefront-safe |
| 37 | + out of the box. Rejected: breaks retries/failure transport on async transports for all |
| 38 | + hosts, imposes a non-configurable policy, and is surprising behavior for a reusable |
| 39 | + plugin. |
| 40 | +- **B. Configuration flag (`throw_on_handler_error`).** Swallow by default, rethrow when |
| 41 | + enabled. Rejected: pushes an infrastructure decision onto every integrator and keeps |
| 42 | + the wrong default semantics for async hosts that forget to enable it. |
| 43 | +- **C. Catch at the dispatch boundary, classify errors in the handlers.** Chosen — see |
| 44 | + below. Idiomatic Messenger, correct in both sync and async setups, zero configuration. |
| 45 | + |
| 46 | +## Decision |
| 47 | + |
| 48 | +Error handling is split across two layers with distinct responsibilities. |
| 49 | + |
| 50 | +### 1. Message handlers: throw transient errors, record permanent ones |
| 51 | + |
| 52 | +Handlers classify every failure with `Webgriffe\SyliusMailchimpPlugin\Util\MailchimpErrorClassifier::isPermanent()`: |
| 53 | + |
| 54 | +- **Permanent** — retrying can never succeed, so rethrowing is pointless: |
| 55 | + - `ClientException` with HTTP status 400–499, **except** 408 (request timeout) and |
| 56 | + 429 (rate limit); |
| 57 | + - `NotFoundException`, `AudienceNotFoundException`, `MissingCustomerEmailException`, |
| 58 | + `\InvalidArgumentException` (missing configuration or invalid data). |
| 59 | + |
| 60 | + The handler logs the error and, where the entity has a dedicated field |
| 61 | + (`mailchimpCartError`, `mailchimpOrderError`, `mailchimpError`), persists the message |
| 62 | + so the existing `Sync*` commands and the admin can detect and re-drive the sync. |
| 63 | + The handler then returns normally. |
| 64 | + |
| 65 | +- **Transient** — retrying makes sense (Mailchimp 5xx, 408, 429, network/transport |
| 66 | + errors, anything unclassified): the handler logs and **rethrows**. On an async |
| 67 | + transport Messenger retries the message and eventually routes it to the failure |
| 68 | + transport; on the sync transport the exception is contained by layer 2. |
| 69 | + |
| 70 | +The `*RemoveHandler` classes keep their original log-and-rethrow behavior (removals are |
| 71 | +idempotent, retrying is always safe). `MemberSubscriptionUpdateHandler` also still |
| 72 | +throws: it is driven by the Mailchimp webhook, where a 500 response correctly makes |
| 73 | +Mailchimp retry the delivery. |
| 74 | + |
| 75 | +### 2. Enqueuers (dispatch boundary): never let a sync failure escape into the request |
| 76 | + |
| 77 | +Every `MessageBusInterface::dispatch()` call in the `src/Enqueuer/*` classes (Cart, |
| 78 | +Order, Product, Store, Member) is wrapped in `try/catch (\Throwable)` + error log on the |
| 79 | +`mailchimp` channel. The wrap also covers the audience/store resolution done inline by |
| 80 | +some enqueuers. |
| 81 | + |
| 82 | +- **Sync transport (default):** a transient handler exception surfaces at the dispatch |
| 83 | + call, is caught here, and the storefront request completes normally. |
| 84 | +- **Async transport:** `dispatch()` only enqueues and does not throw handler exceptions; |
| 85 | + the catch is inert and the worker gets native retry semantics. |
| 86 | + |
| 87 | +`NewsletterController` is its own dispatch boundary with richer handling, because the |
| 88 | +newsletter AJAX flow must give user feedback: it catches `HandlerFailedException`, |
| 89 | +returns 422 with the compliance message (and resubscribe URL) for |
| 90 | +`ComplianceStateException`, and a generic 500 JSON for anything else. The CLI `Sync*` |
| 91 | +commands intentionally keep no boundary: visible failures are desirable there. |
| 92 | + |
| 93 | +## Consequences |
| 94 | + |
| 95 | +- A Mailchimp outage or API error can no longer break add-to-cart, checkout, profile |
| 96 | + update or registration, regardless of transport configuration. |
| 97 | +- Hosts using an async transport get full Messenger retry + failure-transport semantics |
| 98 | + for transient errors, with no plugin configuration required. |
| 99 | +- Permanent errors are recorded on the affected entity where a field exists and are |
| 100 | + visible in the logs; they do not consume pointless retries. |
| 101 | +- On the sync transport, transient failures are logged but **not** persisted on the |
| 102 | + entity (the exception aborts the handler before any error-field flush survives); |
| 103 | + recovery relies on the logs and the `Sync*` commands. This is accepted. |
| 104 | +- New sync paths must follow the same split: classify in the handler, wrap the dispatch |
| 105 | + in the enqueuer. Do not add catch-all blocks inside handlers. |
| 106 | +- The classification lives in one place (`MailchimpErrorClassifier`); changing what |
| 107 | + counts as permanent (e.g. treating 429 differently) is a one-line policy change. |
0 commit comments