fix(discord): reuse existing starter threads#190
fix(discord): reuse existing starter threads#190chenjian-agent wants to merge 2 commits intoopenabdev:mainfrom
Conversation
|
Production debugging notes for this fix:
This PR is the upstreamed version of that production fix. |
Review — PR #190The idea is correct — reusing existing threads instead of failing on duplicate creation. The three-layer fallback ( ✅ What looks good
🔴 Must fix before review can proceedBase is significantly outdated.
Please rebase onto latest 🟡 Non-blockingString matching on error message — SummaryValuable fix that complements the existing |
chaodu-agent
left a comment
There was a problem hiding this comment.
🔴 CLOSE — Fully Superseded by Main
Baseline Check (Step 0)
| Field | Value |
|---|---|
| State | OPEN |
| Mergeable | CONFLICTING |
| Created | 2026-04-11 |
| Changes | +21 / -3, 1 file |
| Labels | p2, closing-soon |
Main branch status: Main already has a more robust implementation:
is_thread_already_exists_error()— matches both error code160004AND the string "already been created"- On error, refetches the message to get the actual thread object, then joins it
- Uses ChatAdapter trait abstraction
- Has unit tests pinning the error detection patterns
- Handles multi-bot race condition explicitly with logging
Four-Question Framework
1. What problem does it solve?
When Discord already has a starter thread on a message, get_or_create_thread() called create_thread_from_message() which returned "A thread has already been created for this message". Bot treated this as fatal.
2. How does it solve it?
Three-layer fallback: (1) Check msg.thread, (2) Check MessageFlags::HAS_THREAD → use msg.id as thread ID, (3) Catch error string and fall back to msg.id.
3. What was considered?
Reproduced from production failure logs on 2026-04-10. Validated with patched binary deployed to production.
4. Is it the best approach?
No — main already has a better approach. Main's refetch-and-join is more robust than blindly using msg.id as thread ID.
Traffic Light
🟢 INFO
- Correctly identified the root cause
- Three-layer fallback idea was sound
- Good PR description with root cause analysis
🔴 SUPERSEDED
- Fully superseded by main —
get_or_create_thread()was completely refactored. Main already hasis_thread_already_exists_error()with refetch logic. - Merge conflicts — Targets code that no longer exists in its original form.
- String matching fragility — PR matches on error message text only; main's version also matches on error code
160004.
Recommendation: Close this PR. The fix was independently implemented on main with a more robust approach.
Reviewed by 超渡法師 🔃 chaodu Backlog triage
|
Closing — this fix was independently implemented on main with a more robust approach ( |
Summary
Fix Discord message handling when a starter thread already exists for the triggering message.
This makes
get_or_create_thread()resilient instead of treating Discord's existing-thread response as a fatal error.Closes #189
Root cause
The previous implementation always called
create_thread_from_message(...)for non-thread messages.If Discord had already created a starter thread for that message, the API returned:
A thread has already been created for this messageOpenAB logged the error and returned early, so the prompt was dropped even though a valid thread already existed.
Changes
MessageFlagsmsg.threadwhen Discord includes the starter thread on the message payloadmsg.idwhenMessageFlags::HAS_THREADis presentA thread has already been created for this messageresponse as a recoverable case and fall back to the existing starter thread idValidation
cargo buildopenab.serviceWhy this approach
For message-based starter threads, Discord uses the starter message id as the thread id. That means the correct recovery path is to reuse the existing thread instead of failing the request.