feat(twitch): use Promise.allSettled for per-event subscription error logging

[LucasSantana-Dev]

Summary

• Use Promise.allSettled() instead of Promise.all() in both session_welcome handler and refreshSubscriptions()
• Add per-event error logging to identify which specific subscription failed
• Gracefully handle partial failures instead of aborting on first failure

Changes

• Added EVENT_LABELS constant mapping subscription indices to event type names
• Replaced Promise.all().catch() with Promise.allSettled() + per-result checking
• Each rejected subscription now logs: Twitch EventSub: {event_type} subscription failed

Verification

Diff reviewed; code changes replace generic error handling with granular per-event logging. Both Promise.all() instances replaced with allSettled().

Closes #1529

---

Summary by cubic

Switch Twitch EventSub subscriptions to Promise.allSettled with per-event error logging. Handles partial failures and prevents false failure logs on reconnect, aligning with Linear #1529.

• Bug Fixes
  • Replace Promise.all with Promise.allSettled in session_welcome and refreshSubscriptions; log per-event failures using EVENT_LABELS.
  • Count already-subscribed broadcasters as success; throw in subscribeToEvent when none succeed to enable per-event failure logging.
  • Capture sessionId in a local const for the welcome handler and normalize allSettled reason typing.

^{Written for commit e06b7dd. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Improved Twitch event subscription reliability by allowing successful subscriptions to continue even when individual event registrations fail.
  • Added clearer error reporting to identify which event subscription could not be registered or refreshed.
  • Subscription attempts now correctly report a failure when none of the requested broadcaster subscriptions succeed.

[github-actions]

Failed to generate code suggestions for PR

[coderabbitai]

📝 Walkthrough

Walkthrough

Twitch EventSub registration now records individual subscription failures without aborting sibling registrations. Broadcaster subscription setup also counts successful attempts and throws when no discovered broadcaster can be subscribed.

Changes

EventSub subscription reliability

Layer / File(s)	Summary
Event labels and per-subscription failure logging packages/bot/src/twitch/eventsubClient.ts	session_welcome and refreshSubscriptions() use Promise.allSettled() and log rejected subscriptions with their corresponding EventSub labels.
Aggregate broadcaster subscription failure detection packages/bot/src/twitch/eventsubSubscriptions.ts	Subscription setup counts successful broadcaster registrations and throws when all attempts fail for a non-empty user list.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TwitchEventSubClient
  participant TwitchAPI
  participant Logger

  TwitchEventSubClient->>TwitchAPI: Register four EventSub subscriptions
  TwitchAPI-->>TwitchEventSubClient: Promise.allSettled results
  loop Each settled result
    alt Subscription rejected
      TwitchEventSubClient->>Logger: Log event label and rejection reason
    end
  end

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements Promise.allSettled in both session_welcome and refreshSubscriptions with per-event labels, matching issue #1529.
Out of Scope Changes check	✅ Passed	The extra subscribeToEvent failure rejection supports the requested logging fix and no unrelated changes are evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: switching Twitch EventSub handling to Promise.allSettled with per-event error logging.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1529-twitch-promise-allsettled

---

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

1 issue found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/bot/src/twitch/eventsubClient.ts">

<violation number="1" location="packages/bot/src/twitch/eventsubClient.ts:162">
P2: Per-event failure logging will miss most actual Twitch subscription failures because `subscribeTo*` resolves even when `createSubscription()` returns `false`. Consider returning/throwing failure details from the subscription helpers, or logging the event type at the `createSubscription` failure site, so HTTP/fetch failures are labeled too.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[LucasSantana-Dev]

Comment on review thread (PR #1749)

Verdict: Issue is valid ✓

Confirmed — subscribeToEvent() never rejects, so the Promise.allSettled per-event handlers never trigger.

Root cause: When createSubscription() returns false (HTTP/network error), the error IS logged at line 164 of eventsubSubscriptions.ts. But subscribeToEvent() silently continues the loop and resolves with void. The promise in eventsubClient.ts line 150 never rejects, so the per-event error log never runs — it's dead code.

Impact: The PR adds per-event error logging but doesn't achieve its goal because the promises never reject.

Recommended fix: Make subscribeToEvent() reject if no subscriptions succeeded, or return a success indicator so the caller knows the whole event subscription failed. This is a minimal one-line fix and aligns with the PR's stated purpose.

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/bot/src/twitch/eventsubSubscriptions.ts (1)

84-105: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Count existing subscriptions as successful.

On a reconnect migration, existing EventSub subscriptions and subscribedIds are retained. Every ID is skipped, leaving successCount at zero and causing a false per-event failure log despite active subscriptions. The same occurs when one ID is already subscribed and newly discovered IDs fail. Track an existing-or-new successful subscription, rather than only newly created ones.

Proposed fix

-    let successCount = 0
+    let hasSubscribedBroadcaster = false
     for (const broadcasterUserId of userIds) {
-        if (subscribedIds.has(broadcasterUserId)) continue
+        if (subscribedIds.has(broadcasterUserId)) {
+            hasSubscribedBroadcaster = true
+            continue
+        }
         const ok = await createSubscription(
             broadcasterUserId,
             token,
             sessionId,
             clientId,
             type,
             version,
             conditionKey,
         )
         if (ok) {
             subscribedIds.add(broadcasterUserId)
-            successCount++
+            hasSubscribedBroadcaster = true
         }
     }

-    if (successCount === 0 && userIds.length > 0) {
+    if (!hasSubscribedBroadcaster) {
         throw new Error(
             `Twitch EventSub: failed to subscribe to any broadcasters for ${type}`,
         )
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bot/src/twitch/eventsubSubscriptions.ts` around lines 84 - 105,
Update the subscription loop in the event subscription migration logic to count
IDs already present in subscribedIds as successful before skipping them, while
continuing to count newly created subscriptions when createSubscription
succeeds. Use this combined success count for the failure check so retained
subscriptions do not trigger a false error.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bot/src/twitch/eventsubClient.ts`:
- Around line 162-169: The rejected promise reasons in the Promise.allSettled
handlers should not be passed as any to errorLog. In both result-processing
branches around the subscription handling logic, normalize result.reason to
unknown or an Error before assigning it to the error field, preserving the
existing failure context and satisfying type-safe linting.

---

Outside diff comments:
In `@packages/bot/src/twitch/eventsubSubscriptions.ts`:
- Around line 84-105: Update the subscription loop in the event subscription
migration logic to count IDs already present in subscribedIds as successful
before skipping them, while continuing to count newly created subscriptions when
createSubscription succeeds. Use this combined success count for the failure
check so retained subscriptions do not trigger a false error.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bd74a72d-239e-494f-af7b-90bf4b70ffa5

📥 Commits

Reviewing files that changed from the base of the PR and between 2e5fd94 and 6facb36.

📒 Files selected for processing (2)

• packages/bot/src/twitch/eventsubClient.ts
• packages/bot/src/twitch/eventsubSubscriptions.ts

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

<sub>Requires human review: Refactors error handling in Twitch EventSub subscriptions. Functional change in a critical integration path; requires human review for correctness and potential side effects.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 1 file (changes from recent commits).

<sub>Auto-approved: Switches to Promise.allSettled for per-event error logging in Twitch EventSub; adds partial-failure handling. Safe defensive change with low risk.

Re-trigger cubic</sub>

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud