Skip to content

Bug: Session-scoped reminders hang 1h when Discord reply fails #1372

Description

@petabridge-netclaw

Bug: Session-scoped reminders hang for 1h when Discord reply fails

Summary

When a Discord reply fails in DiscordSessionBindingActor.HandleOutputReceivedAsync, the SafeReplyAsync method catches the exception, logs it, and returns early — but the caller unconditionally sets _deliveredThisTurn = true on the next line assuming success. Since the early return skips that line, _deliveredThisTurn stays false.

When TurnCompleted arrives, the code at line 1164 checks _deliveredThisTurn before publishing ReminderDeliveryObserved:

if (!string.IsNullOrWhiteSpace(completed.SourceReminderId) && _deliveredThisTurn)
{
    Context.System.EventStream.Publish(new ReminderDeliveryObserved(...));
}

Because the flag is false, the event is never published. The ReminderExecutionActor is sitting in WaitForDeliveryObservationAsync() with a hardcoded 1-hour timeout (DeliveryObservedTimeout = TimeSpan.FromHours(1)). It waits the full hour, times out, and marks the execution as failed.

Meanwhile, ReminderManagerActor still has the reminder tracked in _activeExecutions. Every subsequent reminder fire gets reminder_skipped_duplicate_execution because the first execution is still "in flight" (TCS never resolved).

Impact

A single Discord API error can block a recurring session-scoped reminder for up to 1 hour, and all reminder firings in that window are silently skipped.

Root Cause

SafeReplyAsync swallows exceptions internally but returns void. The callers in HandleOutputReceivedAsync assume success:

DiscordSessionBindingActor.cs:1103-1105

case TextOutput textOutput:
    await SafeReplyAsync(textOutput.Text);   // throws -> early return
    _deliveredThisTurn = true;               // NEVER reached
    break;

DiscordSessionBindingActor.cs:1108-1110 (same for ErrorOutput)

case ErrorOutput error:
    await SafeReplyAsync($":warning: {error.Message}");
    _deliveredThisTurn = true;  // Same gap
    break;

Suggested Fix

Ensure _deliveredThisTurn = true is set after the delivery attempt regardless of success.

Option A (minimal): Always set the flag after the call — the delivery attempt was made:

case TextOutput textOutput:
    try { await SafeReplyAsync(textOutput.Text); } catch { /* already logged */ }
    _deliveredThisTurn = true;  // Always set
    break;

Option B: Make SafeReplyAsync return bool and check it at the call site.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions