Skip to content

Defer component interactions on arrival to meet Discord's 3s ack window#51

Merged
Firestone82 merged 6 commits into
masterfrom
claude/fix-discord-interaction-timeout-MdkmL
May 18, 2026
Merged

Defer component interactions on arrival to meet Discord's 3s ack window#51
Firestone82 merged 6 commits into
masterfrom
claude/fix-discord-interaction-timeout-MdkmL

Conversation

@Firestone82
Copy link
Copy Markdown
Owner

Summary

Refactors component interaction handling across the codebase to defer interactions immediately upon arrival in a spawned task, rather than deferring them serially in the main event loop. This ensures the 3-second Discord acknowledgment window is always met, even when the main handler is busy with locks, I/O, or slow operations like yt-dlp probes.

Key Changes

  • New DeferredInteractionStream service (src/service/interaction_service.rs):

    • Wraps ComponentInteractionCollector to buffer clicks and defer each one immediately on arrival via tokio::spawn.
    • Provides next() and next_within(timeout) methods for serial consumption of already-acked interactions.
    • Consumers receive interactions that have been silently deferred, so they must use message.edit(), create_followup(), or edit_response() — not create_response().
  • Gather service refactor (src/service/gather_service.rs):

    • Replaces ComponentInteractionCollector + tokio::select! with DeferredInteractionStream.
    • Adds reconsidering field to GatherState to track users who opted out and were disconnected, enabling them to rejoin and un-opt themselves.
    • Introduces CheckInOutcome enum to clarify interaction handler return values (None, Refresh, Cancel).
    • Simplifies interaction response handling by using helper send_ephemeral() for error messages.
    • Removes shard clone and manual CreateInteractionResponse construction in favor of deferred updates.
  • Queue pagination (src/commands/music/cmd_queue.rs):

    • Migrates to DeferredInteractionStream.
    • Adds page indicator button to navigation row.
    • Simplifies interaction loop and error handling.
  • Reputation list (src/commands/reputation/cmd_list.rs):

    • Migrates to DeferredInteractionStream.
    • Replaces ComponentInteractionCollector stream with deferred interaction handling.
    • Uses message.edit() instead of create_response() for updates.
  • History command (src/commands/music/cmd_history.rs):

    • Migrates to DeferredInteractionStream.
    • Simplifies timeout and interaction handling logic.
  • Picker service (src/service/picker_service.rs):

    • Migrates to DeferredInteractionStream.
    • Replaces tokio::time::timeout wrapper with next_within().
    • Uses create_followup() for non-author error messages.
  • Play command (src/commands/music/cmd_play.rs):

    • Moves "track added" confirmation before player lock acquisition to provide immediate feedback while yt-dlp probes the URL.
  • Voice handler (src/handlers/voice_handler.rs):

    • Integrates with reconsidering flag to route opt-out rejoiners through auto_arrived.
  • Service module (src/service.rs):

    • Declares new interaction_service module.

Implementation Details

  • Deferred interactions are acked via defer() (a silent DeferredUpdateMessage), so subsequent state changes must go through message edits or followups, not response creation.
  • The forwarder task spawns a new task per click to defer it immediately, preventing any single slow handler from blocking the ack window for subsequent clicks.
  • DeferredInteractionStream::drop() aborts the forwarder task to clean up resources.
  • The reconsidering set enables a two-phase opt-out flow: when a user marks "I'm out", they are disconnected; if they rejoin, the voice handler routes them through auto_arrived, where the check-in loop consults reconsidering to un-opt them and restore their expected status.

https://claude.ai/code/session_01DsM59VsSctY6UGfwTtSjQe

claude added 3 commits May 18, 2026 10:44
Discord expires a component interaction token 3 seconds after the click.
When multiple users hit buttons in quick succession — pagination on
queue/history/rep_list, the search picker, or the gather check-in — the
handlers processed them serially and ran significant async work (player
lock, yt-dlp probe, message edits) before acking. Any click that landed
in the buffer during that slow handler was already expired by the time
we got to it, so Discord rejected the response and the user saw "This
interaction failed".

Introduce DeferredInteractionStream in service/interaction_service.rs.
It wraps ComponentInteractionCollector, and for every click that arrives
it spawns a task that immediately calls defer(), then hands the already-
acked interaction back to the consumer for serial processing. The ack
no longer waits in line behind whatever the main loop is doing.

Refactor every component-interaction site onto it: picker_service,
cmd_queue, cmd_history, cmd_list, and gather_service (both the pregather
phase and the check-in phase). Update-message flows now go through
message.edit; ephemeral rejections go through create_followup since the
interaction is already acked.

Separately: !play now sends the "Added to queue" embed even when the
queue is empty. Previously it was skipped on the assumption that
NowPlaying would cover it, but that meant the user stared at a blank
"Bot is thinking…" while yt-dlp probed the URL. Send the confirmation
first, then let NowPlaying follow when next_track resolves the input.
Pressing "I'm out" used to leave the user sitting in the gathering
voice channel — confusing both for them and for the rest of the
group, since the embed says they're not coming but they're still
visibly there. Disconnect them on opt-out so the gesture matches
the state.

If they reconsider and rejoin the same channel, treat the rejoin as
their check-in: un-opt them and stamp the current arrival time
(late if past the grace period). A new `reconsidering` set on
GatherState arms this — voice_handler routes both extra_expected
joiners and reconsidering joiners through auto_arrived, and the
check-in loop tells them apart by draining `reconsidering` as it
processes the batch.

The disconnect itself is spawned so the Discord API call can't push
the next button click past the 3-second ack window.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cf119ff6d2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/service/interaction_service.rs Outdated
Comment on lines +45 to +50
tokio::spawn(async move {
if let Err(error) = ic.defer(&http).await {
tracing::debug!("Failed to defer component interaction: {:?}", error);
return;
}
let _ = tx.send(ic);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve interaction order in deferred forwarder

Forwarding each click through its own tokio::spawn means interactions are enqueued after defer() completes, so delivery order becomes network-latency order instead of click order. In fast multi-click flows (for example queue pagination or gather button toggles), this can apply actions out of sequence and leave the message in a state that doesn't match what users clicked. The previous collector path preserved stream order, so this is a behavioral regression.

Useful? React with 👍 / 👎.

Comment on lines +255 to +258
QueueEmbed::TrackAdded(&track)
.to_embed()
.send_context(ctx, true, Some(30))
.await?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Emit queue success after add operation succeeds

The command now sends QueueEmbed::TrackAdded before add_track_to_queue runs. If queue insertion/playback startup fails (e.g., voice/playback backend errors), users receive a success confirmation followed by an error, which is incorrect state reporting. This regression is newly visible on the idle-path too because the prior player.is_playing guard was removed.

Useful? React with 👍 / 👎.

claude added 3 commits May 18, 2026 15:19
interaction_service: remove the inner tokio::spawn per defer — doing
defers inline in the forwarder loop preserves the original click order.
Each defer is ~100-300 ms, so a burst of several rapid clicks still
completes well within Discord's 3-second ack window.

cmd_play: move QueueEmbed::TrackAdded to after add_track_to_queue
succeeds so a playback startup error can't be preceded by a success
confirmation. NowPlaying still follows once next_track resolves the
input, keeping the idle-queue UX intact.
add_track_to_queue ran kick_off_playback internally, and kick_off
chains into next_track which sends the NowPlaying embed. After
moving the TrackAdded confirmation to after add_track_to_queue
returns (to avoid showing success before an error), the user saw
"Now Playing" before "Added to queue" on an idle queue.

Split the player API: push_track is the infallible queue insert,
kick_off_playback (now pub) is the fallible playback start. cmd_play
pushes, sends TrackAdded while still holding the lock so order is
deterministic, then kicks off playback — TrackAdded lands first and
a kick-off failure still aborts before showing success.
…fter stop

- cmd_play: send a "Queuing…" embed immediately after ctx.defer() for
  direct YouTube/Spotify URLs so the user gets instant feedback before
  the metadata fetch completes. Text searches still go straight to the
  picker (that IS the immediate response).

- cmd_stop: QueueHandler only starts the 5-minute inactivity timer when
  the queue empties naturally via a TrackEnd event. When !stop is called
  the track is halted programmatically and QueueHandler sees is_playing=false,
  exiting early without spawning the timer. Spawn it in cmd_stop instead,
  mirroring the same cancel-flag / is_playing guard logic.

https://claude.ai/code/session_01DsM59VsSctY6UGfwTtSjQe
@Firestone82 Firestone82 merged commit b8eed0b into master May 18, 2026
3 checks passed
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.

2 participants