Fix Discord/DiscordDm reporting success on partial failure#7
Merged
Conversation
…ilure Both used Any() across per-recipient results, so if 1 of N webhooks/DMs succeeded the whole post was reported as a success and the orchestrator's failure warning never logged. Twilio and VoIP.ms use All() for the same multi-recipient pattern; align Discord and DiscordDm with that.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
77
to
+79
| var tasks = _settings.UserIds.Select(userId => SendToUserAsync(userId, content, embed, label, imageBytes)); | ||
| var results = await Task.WhenAll(tasks); | ||
| return results.Any(r => r); | ||
| return results.All(r => r); |
There was a problem hiding this comment.
CLAUDE.md compliance: this PR is a behavior-changing bug fix (here and in Services/DiscordService.cs#L67-L71, aggregate success now requires all recipients to succeed instead of any), but includes no README.md update.
Root CLAUDE.md, Non-Negotiable Rule #1 ("Always update README.md"):
Any change — new feature, new service, new config field, behavior change, bug fix — must include a corresponding README.md update. ... If unsure which section to update, add a note under
## Recent Changesat the bottom.
The README's ## Recent Changes section already documents comparable backend bug fixes (e.g. the VoIP.ms GET-vs-POST fix, the SPC MCD polygon parsing fix) — please add a similar bullet describing this fix.
Per CLAUDE.md Non-Negotiable Rule #1, caught by the claude-code-review bot's inline comment on this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DiscordService.SendAsyncandDiscordDmService.SendToAllUsersAsyncboth usedresults.Any(r => r)across per-recipient send results, so if only 1 of N webhooks/DM recipients succeeded, the whole post was reported as an overall success — the orchestrator's "Delivery was not successful" warning never fires for a real partial failure.TwilioServiceandVoipMsServiceuse.All(r => r)for the structurally identical multi-recipient pattern. Aligned Discord and DiscordDm with that.Test plan
PostAlertAsyncnow returnsfalseand the failure is logged, instead of silently reporting success