Ivan Ball-llovera

↑↓ to navigate Enter to open "…" all these words ANDOR to combine

Architecture Decision Record

ADR-096: Best-Effort Side-Effect Contract

Status

Accepted (2026-08-23). Revised 2026-08-31. Revised 2026-09-19. Revised 2026-09-25 (the ADR-054 cross-reference is re-anchored onto that record's current best-effort trade-off). Revised 2026-10-01. Revised 2026-10-06: adoption is thirteen call sites with the ADC points award, and every Store Catalog controller evicts both catalog tags.

Context

A command that has already committed often has follow-up work attached to it: evict the output-cache entries the write invalidated, broadcast the new state to a live channel, send the notification the user is waiting for. That work can fail on its own, and when it does the question is not whether to retry but what the failure is allowed to do to the caller. Turning it into an exception would roll back, retry or 500 an operation whose real work already succeeded.

Five records each answer that question locally, for their own feature, and each answer is right: ADR-024 makes a push delivery failure non-fatal and records MarkAsFailed instead (024-push-notifications.md:65-67), ADR-026 makes cross-service cache eviction best-effort so a broken eviction store cannot dead-letter a coherence hint, ADR-076 degrades a data-subject export per section rather than failing the package (076-data-subject-export.md:82-83), ADR-091 composes the reset email in the handler and delivers it best-effort, "awaited and its failure caught, logged and swallowed" (091-cache-backed-password-reset.md:81-86), and ADR-054 makes compensation best-effort per order line (054-saga-compensation-and-reconciliation.md:252-263). What none of them decides is the policy: which failures may be swallowed at all, at what severity, whether cancellation counts as one of them, and how a swallow is made visible to somebody who is not reading the log. Answered per call site, that produces a repo full of hand-rolled catch (Exception) blocks, each choosing its own severity, its own treatment of cancellation and its own decision to count nothing. ADR-041 records the counter this record's helper emits and notes that it is wired to no alert (041-observability-and-telemetry.md:235-240, :258-261), but it records the instrument, not the contract behind it.

Decision

One framework helper defines the contract, and a swallow that does not go through it is a deliberate, documented exception.

  • BestEffort.ExecuteAsync(operation, logger, action, cancellationToken = default), a static helper in the Application layer (MMCA.Common/Source/Core/MMCA.Common.Application/Services/BestEffort.cs:45-49), runs the side effect and absorbs its failure.
  • The action is awaited, not fire-and-forget. The helper awaits action(cancellationToken) (:57), so the side effect completes (or fails) before the caller continues; nothing is left as an orphan task racing the response.
  • A failure produces exactly one Warning. BestEffortLog.DispatchFailed is a source-generated [LoggerMessage] at Warning (:81-84) reading "Best-effort operation '{Operation}' failed and was swallowed; the caller's outcome is unaffected" (:83), with the exception attached. The catch is deliberately broad, with the reason written into the code: the whole contract is that nothing the side effect throws reaches the caller (:65-71).
  • A failure produces exactly one metric increment. besteffort.dispatch.failed is a Counter<long> on its own meter, MMCA.Common.BestEffort (:102, instrument at :107-115), incremented with an operation tag (:115). It is a meter of its own rather than a counter folded into MMCA.Common.Cqrs, because best-effort dispatch is not part of the CQRS pipeline and an operator can drop or keep it independently of the RED metrics (:93-97). The Aspire service defaults subscribe it (MMCA.Common/Source/Hosting/MMCA.Common.Aspire/Extensions.Telemetry.cs:313).
  • The operation name is a low-cardinality constant. It becomes a metric tag (:22), so call sites pass a const or a fixed prefix plus a value from a small fixed set. A blank name throws ArgumentException and a null logger or action throws ArgumentNullException (:51-53): the helper swallows the side effect's failures, never its own caller's bugs.
  • Cancellation is not swallowed. An OperationCanceledException raised while the caller's own token is cancelled is rethrown (:59-63), so a host shutdown or an abandoned request unwinds promptly instead of being recorded as a spurious side-effect failure. A cancellation that is not the caller's (an inner timeout) is a genuine failure of the side effect and is swallowed like any other.
  • Post-commit work passes CancellationToken.None on purpose. The write has committed, so its follow-up must outlive a caller that has already walked away: the framework's own output-cache eviction helper passes it explicitly (MMCA.Common/Source/Presentation/MMCA.Common.API/Caching/OutputCacheEvictionExtensions.cs:78-92, the token at :90; the ADC submit and moderation broadcasts take the parameter's default for the same reason, .../SubmitQuestionHandler.cs:197-200, the call closing at :235, and .../ModerateQuestionHandler.cs:157, while the three ADC domain-event handlers pass their own cancellationToken, .../SessionQuestionUpvoteChangedHandler.cs:84, .../LivePollVoteChangedHandler.cs:83 and .../SessionQuestionSubmittedPointsHandler.cs:102, and the hosted bookmark eviction processor passes its stoppingToken, .../Caching/BookmarkCacheEvictionProcessor.cs:77).
  • The contract is pinned by tests. BestEffortTests covers the transparent success path, token passthrough, one-Warning-per-failure, the operation-tagged increment observed through a MeterListener, the rethrow of the caller's cancellation, the swallow of a non-caller cancellation, and argument validation (MMCA.Common/Tests/Core/MMCA.Common.Application.Tests/Services/BestEffortTests.cs:15-141).

Adoption today is thirteen call sites: seven in ADC Engagement, five in Store, and the framework's own eviction helper. The ADC seven are the live-channel drain worker, whose operation name is the prefix live-channel-publish: plus the work item's event name and whose own catch turns the rethrown cancellation into a quiet stop (MMCA.ADC/Source/Modules/Engagement/MMCA.ADC.Engagement.Infrastructure/Live/LiveChannelPublishProcessor.cs:36, :45-58, :60-65); three session-question broadcasts, session-question-submit-broadcast (.../SessionQuestions/UseCases/Submit/SubmitQuestionHandler.cs:41, call at :206, which enqueues onto the live-channel publish queue at :217-218 rather than publishing inline), session-question-moderation-broadcast (.../UseCases/Moderate/ModerateQuestionHandler.cs:31, call at :138) and session-question-upvote-broadcast (.../DomainEventHandlers/SessionQuestionUpvoteChangedHandler.cs:45, call at :52); the poll-results broadcast livepoll-results-broadcast (.../LivePolls/DomainEventHandlers/LivePollVoteChangedHandler.cs:44, call at :51); and the cross-host cache eviction bookmark-cache-evict-broadcast (MMCA.ADC/Source/Modules/Engagement/MMCA.ADC.Engagement.Infrastructure/Caching/BookmarkCacheEvictionProcessor.cs:49, call at :62-77, a hosted processor that drains the signal the domain-event handler raises); and the question-asked points award session-question-points-award, which runs after the question's transaction commits so a failed award never fails the committed question (MMCA.ADC/Source/Modules/Engagement/MMCA.ADC.Engagement.Application/Points/DomainEventHandlers/SessionQuestionSubmittedPointsHandler.cs:59, call at :82-102). The Store five are a checkout display label, checkout-customer-name, resolved by the checkout preflight outside the transaction so an unreachable Identity leaves the name null instead of failing an otherwise valid checkout (MMCA.Store/Source/Modules/Sales/MMCA.Store.Sales.Application/ShoppingCarts/UseCases/CheckOut/CheckOutPreflight.cs:77-87, the operation name at :78, the outside-the-transaction reasoning at :17-24); three inventory label fetches sharing the operation name inventory-catalog-labels, so a Catalog service that drops out leaves rows with whatever labels they had (.../Inventory/UseCases/Create/CreateInventoryItemHandler.cs:68, .../Inventory/UseCases/BulkSet/BulkSetInventoryHandler.cs:67 and .../Inventory/UseCases/Set/AdjustInventoryHandler.cs:66-67); and the post-save eviction broadcast review-anonymize-cache-evict-broadcast raised after a customer erasure anonymizes reviews, where a broker fault must not fail an erasure that has already committed (MMCA.Store/Source/Modules/Catalog/MMCA.Store.Catalog.Application/Reviews/IntegrationEventHandlers/CustomerErasedHandler.cs:62, call at :109-115). Four of those five are pre-commit reads rather than post-commit follow-ups (the create fetch runs before base.PersistAsync at CreateInventoryItemHandler.cs:82, the set fetch before SaveChangesAsync at AdjustInventoryHandler.cs:81): the contract is about what a failure is allowed to do to the caller, not about where in the handler the work sits. The thirteenth is the framework's own multi-tag eviction helper, OutputCacheEvictionExtensions.TryEvictTagsAsync, whose operation name is the constant prefix output-cache-evict: plus the tag being evicted (MMCA.Common/Source/Presentation/MMCA.Common.API/Caching/OutputCacheEvictionExtensions.cs:34, :78-92). Store Catalog reaches it from seven controllers, each passing fixed low-cardinality cache tags, and every one of them evicts catalog:products and catalog:categories together: Controllers/CategoriesController.cs:174, ProductsController.cs:172, ProductVariantsController.cs:206, ProductImagesController.cs:221, ReviewsController.cs:324, ReviewModerationController.cs:166 and ProductAttributesController.cs:154.

One swallow deliberately stays hand-rolled. The framework's own OutputCacheEvictionHandler hand-rolls the same swallow-log-count shape against cache.eviction.failed on the MMCA.Common.OutputCache meter (MMCA.Common/Source/Presentation/MMCA.Common.API/Caching/OutputCacheEvictionHandler.cs:51-62): it is the one documented non-reuse of this helper inside the framework, and ADR-026 records the rationale (026-caching-strategy.md:585-590).

Store's AddVariantHandler used to be the second hand-rolled case, and it now shows what the contract says to do when a side effect is too important to swallow: stop swallowing it. It no longer catches anything and no longer publishes inline. After SaveChangesAsync populates the database-generated ProductVariantId, it schedules a durable ADR-114 internal command, PublishProductVariantChangedInternalCommand, carrying the ProductId and that ProductVariantId, with CancellationToken.None so the follow-up outlives a caller that has walked away (MMCA.Store/Source/Modules/Catalog/MMCA.Store.Catalog.Application/Products/UseCases/AddVariant/AddVariantHandler.cs:92-95). The command is ITransactional (AddVariantCommand.cs:25), so the scheduler only enrolls the row and the transactional pipeline saves it just before the commit: the row commits with the variant (AddVariantHandler.cs:77, :79-82, :97-98). The scheduled row is the durable record, so a broker fault retries with backoff instead of stranding the variant without inventory; an inline publish left a window in which a crash between the commit and the publish lost the event outright, and the outbox could not help because the row only lands there once PublishAsync has been reached. What survives is much narrower: only a failure to write the row itself loses the event. That failure arrives as a Result, not an exception, and is still isolated from the caller's outcome (the variant is committed, and a client retry with a null SKU would create a duplicate), and it is still logged at Error with both ids, because it is the case where an admin has to create the inventory record by hand (:99-100, the [LoggerMessage] at :110-113).

Rationale

  • One policy beats five local leniencies. Each feature record is still right about its own degradation; what they could not each decide is the shape of the swallow. A single helper fixes severity, cardinality and the cancellation rule once, so a new post-commit side effect inherits them instead of re-litigating them.
  • A swallowed failure has to be countable. A Warning in a log nobody reads is how a side effect quietly stops working for weeks. The counter turns "the broadcast has been failing since Tuesday" into a question a dashboard can answer, which is the only thing that makes swallowing defensible.
  • Cancellation is not a failure. Swallowing it would turn an orderly shutdown into a burst of spurious warnings and a metric spike, and would let a stopping host keep doing work it was told to stop. Rethrowing keeps shutdown a shutdown.
  • Awaiting keeps the failure attributable. A detached task would still fail, just later, off the request's context and without the logger scope that names what it was doing.
  • Fixing the severity at Warning is a filter, not a limitation. A swallow that genuinely deserves Error, with ids an operator must act on, is evidence the work is not best-effort. AddVariantHandler was exactly that case, and the answer was to make the work durable rather than to keep it outside the helper: the publish became a scheduled internal command, and only the narrow failure to record that command still logs at Error.

Trade-offs

  • Nothing gates use of the helper. There is no fitness rule, analyzer or architecture test that fails a build for a hand-rolled catch (Exception) that should have been a BestEffort call; the helper is a convention backed by review. The only inventory is a search, which is how the thirteen call sites and the one remaining hand-rolled swallow above were enumerated.
  • The Warning carries the operation name and the exception, nothing else. No entity id, no correlation payload beyond the ambient scope. SubmitQuestionHandler records that cost explicitly: the question id is one log line earlier, not in the best-effort warning (.../Submit/SubmitQuestionHandler.cs:200-202).
  • The counter is failure-only and alerts on nothing. A healthy system emits zero, and zero is indistinguishable from a host that never wired the meter. ADR-041 puts it in exactly that gap (041-observability-and-telemetry.md:258-261).
  • The meter name is a duplicated literal. MMCA.Common.Aspire subscribes it by string because that package has no reference to Application (BestEffort.cs:89-92, Extensions.Telemetry.cs:313), so a rename has to move in two places or the metric silently stops being exported.
  • A swallow is still a loss. The helper decides that the caller does not see the failure; it does not make the side effect happen. A cache entry heals on its own TTL, but a lost broadcast never replays, and callers whose loss is unrecoverable have to say so themselves.
  • Two "best effort" counters exist. cache.eviction.failed and besteffort.dispatch.failed count the same shape of event on different meters, one per the ADR-026 non-reuse and one from this helper, so an operator asking "what is silently failing" has two places to look.

Revision (2026-10-01)

No decision or rationale changed; the adoption inventory and several statements about it are corrected. Adoption is twelve call sites, not eleven: Store's inventory set path is a fifth Store site, a third inventory-catalog-labels fetch (MMCA.Store/Source/Modules/Sales/MMCA.Store.Sales.Application/Inventory/UseCases/Set/AdjustInventoryHandler.cs:66-67). The checkout-customer-name call lives in the checkout preflight, not the handler (.../ShoppingCarts/UseCases/CheckOut/CheckOutPreflight.cs:77-87). Four of the five Store sites are pre-commit reads, not two. Only the ADC submit and moderation broadcasts take the token parameter's default; the two domain-event handlers pass their own token (.../SessionQuestionUpvoteChangedHandler.cs:84, .../LivePollVoteChangedHandler.cs:83) and the bookmark eviction processor passes its stoppingToken (.../BookmarkCacheEvictionProcessor.cs:77). Store Catalog reaches TryEvictTagsAsync from seven controllers, three of them evicting two tags at once (CategoriesController.cs:171). The AddVariantHandler schedule runs inside the ITransactional command and commits with the variant rather than after the commit (AddVariantHandler.cs:76-79, :94-95). Citations refreshed: the meter subscription (MMCA.Common/Source/Hosting/MMCA.Common.Aspire/Extensions.Telemetry.cs:313), SubmitQuestionHandler, AddVariantHandler, the controller lines, and the ADR-024, ADR-026 and ADR-041 cross-references.

Revision (2026-10-06)

No decision or rationale changed; the adoption inventory is corrected again.

  • Adoption is thirteen call sites, not twelve: ADC adds the question-asked points award, session-question-points-award (.../Points/DomainEventHandlers/SessionQuestionSubmittedPointsHandler.cs:59, call at :82-102), which passes its own cancellationToken (:102), so three ADC domain-event handlers do, not two.
  • All seven Store Catalog controllers now evict catalog:products and catalog:categories together; none evicts catalog:products alone (ReviewsController.cs:324).
  • The hand-rolled OutputCacheEvictionHandler paragraph no longer claims the code itself documents the non-reuse: the handler's comment justifies only the broad catch (OutputCacheEvictionHandler.cs:58-59).
  • Anchors re-verified against current source: the ADR-026, ADR-054 and ADR-091 cross-references, AddVariantHandler and the seven controller lines.

ADR-024 (push delivery failure is non-fatal and recorded rather than raised, one of the local leniencies this policy generalizes), ADR-026 (eviction is best-effort, and its OutputCacheEvictionHandler is the framework's one documented non-reuse of this helper), ADR-041 (records besteffort.dispatch.failed, its meter, and that it is wired to no alert), ADR-054 (compensation is best-effort per line, and its one hand-rolled swallow is the same question answered locally), ADR-076 (per-section degradation makes an incomplete package the contract rather than a failure), ADR-091 (the reset email is awaited, caught, logged and swallowed, the shape this helper standardizes), ADR-114 (the durable queue AddVariantHandler now schedules onto, the alternative to swallowing a side effect whose loss is unrecoverable).

← Back to Architecture Decision Records View source on GitHub ↗