Skip to content

Report every TryBeginInvoke failure when a BeginInvokeFailed handler throws [patch] - #88

Open
matt-edmondson wants to merge 2 commits into
mainfrom
fix/66-report-every-begininvoke-failure
Open

matt-edmondson wants to merge 2 commits into
mainfrom
fix/66-report-every-begininvoke-failure

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #66

Problem

DoInvokes raised BeginInvokeFailed in a plain foreach. If a handler threw on one failure, the exception escaped DoInvokes and the remaining failures were discarded. For example, 3 failed actions produced 1 report. It also read the event twice (is null, then ?.Invoke), so if the last handler unsubscribed in between, every failure was dropped and nothing was thrown.

Change

  • Takes one handler snapshot, which is used for both the null check and the calls.
  • Raises each failure in its own try/catch, so a throwing handler still receives every later failure.
  • If any handler call threw, DoInvokes throws an AggregateException at the end. It holds each handler exception, followed by the failure that handler was handling.
  • The XML docs (event remarks and the DoInvokes exception docs) and the README event table now describe this.

Tests


Generated by Claude Code

…throws [patch]

DoInvokes raised BeginInvokeFailed in a plain loop, so a handler that
threw on one failure escaped DoInvokes and discarded the rest. It also
read the event twice, so a last handler unsubscribing in between could
drop every failure without throwing.

Take one handler snapshot, raise each failure in its own try/catch, and
throw an AggregateException at the end holding each handler exception
followed by the failure it was handling.

Fixes #66

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4pcgEyqMdpRoLpnBm
…ity in bounds

Sonar S3776 counted DoInvokes at cognitive complexity 16 (limit 15) once
the per-failure try/catch was added.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4pcgEyqMdpRoLpnBm
@sonarqubecloud

sonarqubecloud Bot commented Nov 1, 2026

Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor Author

SonarCloud findings on this PR:

  • S3776 (cognitive complexity of DoInvokes, 16 > 15): fixed in 9bc6d08. The reporting now lives in ReportBeginInvokeFailures, and Sonar no longer reports it.
  • S3264 ("Remove the unused event BeginInvokeFailed"): left as is, because it's a false positive. The event is still raised. ReportBeginInvokeFailures copies it into a local handler and invokes that copy, which Sonar's rule doesn't trace. The local copy is the point of the fix: the old code read the event once for the null check and again for the invoke, and a handler unsubscribing in between would drop every failure. ThrowingHandlerStillReceivesEveryLaterFailure and the existing handler tests show the event firing. The finding can be marked as a false positive in SonarCloud.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A BeginInvokeFailed handler that throws on one failure silently loses the rest: 3 failed TryBeginInvoke actions, 1 reported

2 participants