Skip to content

[Docs] Fix Defer examples#4030

Merged
iancooper merged 2 commits into
masterfrom
fix_defer_examples
Feb 23, 2026
Merged

[Docs] Fix Defer examples#4030
iancooper merged 2 commits into
masterfrom
fix_defer_examples

Conversation

@iancooper

Copy link
Copy Markdown
Member

The examples for defer need fixing to support the DeferMessageOnError attribute

iancooper and others added 2 commits February 23, 2026 16:00
… attribute

Now that DeferMessageOnErrorAttribute is available, the Kafka and RMQ
DeferOnError samples use the declarative [DeferMessageOnErrorAsync] attribute
instead of manually throwing DeferMessageAction. This aligns with the
DontAckOnError and RejectOnError sample patterns.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… build exclusion

Add 5 new sample project folders (17 projects) to Brighter.slnx that were
missing: KafkaDeferOnError, KafkaDontAckOnError, KafkaTaskQueueWithDLQ,
RMQDeferOnError, and RMQTaskQueueWithDLQ. Also remove the Debug build
exclusion from Paramore.Brighter.Testing.Tests so it builds in all
configurations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@iancooper iancooper requested a review from preardon as a code owner February 23, 2026 16:26
@iancooper iancooper merged commit 5523adb into master Feb 23, 2026
5 of 6 checks passed
@iancooper iancooper deleted the fix_defer_examples branch February 23, 2026 16:26

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No application code in the PR — skipped Code Health checks.

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@claude

claude Bot commented Feb 23, 2026

Copy link
Copy Markdown

PR Review: [Docs] Fix Defer examples

This is a good cleanup. Replacing the manual DeferMessageAction throw + ConcurrentDictionary retry-tracking workaround with the declarative [DeferMessageOnErrorAsync] attribute is the right move now that the attribute exists. The handler code is noticeably simpler and the documentation is more accurate about how the feature actually works.

A few things worth looking at:


README expected output is inaccurate

Both READMEs show this expected output:

Received message #5: Hello # 5
  -> Simulating failure for message #5 (message will be requeued with delay)
Received message #5: Hello # 5
  -> Successfully processed message #5

The second line shows message #5 again, but _messageCount is a static counter that increments on every invocation (including retries). When the requeued message arrives, Interlocked.Increment runs again and the output will say something like message #6: Hello # 5 — the greeting text stays the same (it's payload data from the sender) but the local counter moves forward. The expected output should reflect that:

Received message #5: Hello # 5
  -> Simulating failure for message #5 (message will be requeued with delay)
Received message #6: Hello # 5
  -> Successfully processed message #6

Consider showing the delayMilliseconds parameter

[DeferMessageOnErrorAsync(step: 0)]

The key differentiator between [DeferMessageOnErrorAsync] and [DontAckOnErrorAsync] is the configurable delay before requeue. Using the default (delayMilliseconds: 0) is fine for compilation purposes, but a sample specifically demonstrating Defer behaviour would be more instructive with an explicit delay, e.g.:

[DeferMessageOnErrorAsync(step: 0, delayMilliseconds: 5000)]

The README currently says "The message pump requeues the message with the configured delay (from the subscription's RequeueDelay)" which is a bit ambiguous when the attribute itself also carries a delay parameter — it's worth clarifying whether the attribute delay and the subscription delay are additive, max-of-two, or one takes precedence.


Brighter.slnx change is unrelated to stated purpose

Removing the build restriction from Paramore.Brighter.Testing.Tests:

-    <Project Path="tests/Paramore.Brighter.Testing.Tests/Paramore.Brighter.Testing.Tests.csproj">
-      <Build Solution="Debug|*" Project="false" />
-    </Project>
+    <Project Path="tests/Paramore.Brighter.Testing.Tests/Paramore.Brighter.Testing.Tests.csproj" />

This re-enables the test project for all Debug solution builds, which is separate from fixing Defer examples. If intentional, a brief mention in the PR description would help reviewers understand the intent — the project may have been excluded for a reason.


Overall

The core change is correct and well-motivated. Switching from manual retry-tracking to the declarative attribute is exactly what these samples should show. The issues above are minor (documentation accuracy and one unrelated slnx change) but worth addressing before merge.

DevJonny pushed a commit to DevJonny/Brighter that referenced this pull request Feb 28, 2026
* [Samples] Update DeferOnError samples to use DeferMessageOnErrorAsync attribute

Now that DeferMessageOnErrorAttribute is available, the Kafka and RMQ
DeferOnError samples use the declarative [DeferMessageOnErrorAsync] attribute
instead of manually throwing DeferMessageAction. This aligns with the
DontAckOnError and RejectOnError sample patterns.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* [Chore] Add missing sample projects to solution and fix Testing.Tests build exclusion

Add 5 new sample project folders (17 projects) to Brighter.slnx that were
missing: KafkaDeferOnError, KafkaDontAckOnError, KafkaTaskQueueWithDLQ,
RMQDeferOnError, and RMQTaskQueueWithDLQ. Also remove the Debug build
exclusion from Paramore.Brighter.Testing.Tests so it builds in all
configurations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant