Skip to content

Commit 361dcac

Browse files
committed
fix(telegram): add panic recovery to HandleUpdate and handleChatMessage
- HandleUpdate now recovers from panics in handler callbacks via defer recoverFromPanic() - Logs panic errors and fires OnError to notify users of internal errors - handleChatMessage recovers from panics in agent goroutine to prevent per-chat mutex deadlock — sends error message to user with /new suggestion - 3 new tests: TestHandleUpdate_RecoverFromPanic, _Callback, _Command
1 parent 7aaae36 commit 361dcac

3 files changed

Lines changed: 177 additions & 1 deletion

File tree

cmd/odek/telegram.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ func handleChatMessage(
570570
bot *telegram.Bot,
571571
handler *telegram.Handler,
572572
sessionManager *telegram.SessionManager,
573-
resolved config.ResolvedConfig,
573+
resolved config.ResolvedConfig,
574574
systemMessage string,
575575
log telegram.Logger,
576576
) {
@@ -584,6 +584,16 @@ func handleChatMessage(
584584
}
585585
defer mu.Unlock()
586586

587+
// Recover from panics so a single bad agent run doesn't deadlock the chat.
588+
defer func() {
589+
if r := recover(); r != nil {
590+
log.Error("panic in handleChatMessage", "chat_id", chatID, "panic", r)
591+
reportError(bot, chatID, messageID, fmt.Sprintf(
592+
"Internal error: %v\n\nThe bot is still running. Use /new to start a fresh session.", r,
593+
))
594+
}
595+
}()
596+
587597
// Create a per-chat TelegramApprover for inline keyboard approval.
588598
approver := telegram.NewTelegramApprover(bot, chatID)
589599
handler.SetApprover(chatID, approver)

internal/telegram/handler.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,10 @@ func defaultPhotoHandler(bot *Bot) func(int64, int, []string) (string, error) {
158158
// ─── Update Routing ───────────────────────────────────────────────────────
159159

160160
// HandleUpdate routes an incoming Telegram update to the appropriate handler.
161+
// Recovers from panics in handler callbacks to prevent a single bad update
162+
// from crashing the entire bot loop.
161163
func (h *Handler) HandleUpdate(upd Update) {
164+
defer h.recoverFromPanic("HandleUpdate", upd.ID)
162165
switch {
163166
case upd.Message != nil:
164167
h.handleMessage(upd.Message)
@@ -169,6 +172,20 @@ func (h *Handler) HandleUpdate(upd Update) {
169172
}
170173
}
171174

175+
// recoverFromPanic catches panics in handler callbacks, logs them, and fires
176+
// OnError if configured. Use as: defer h.recoverFromPanic("method", id).
177+
func (h *Handler) recoverFromPanic(method string, updateID int) {
178+
if r := recover(); r != nil {
179+
h.log.Error("panic recovered", "method", method, "update_id", updateID, "panic", r)
180+
if h.OnError != nil {
181+
// Try to extract a chat ID from the panic context, but
182+
// we don't have it here — use 0 and let the callback
183+
// decide how to handle it.
184+
h.OnError(0, fmt.Errorf("telegram: panic in %s (update %d): %v", method, updateID, r))
185+
}
186+
}
187+
}
188+
172189
// handleMessage routes a single message based on content type and permissions.
173190
func (h *Handler) handleMessage(msg *Message) {
174191
if msg.Chat == nil || msg.From == nil {
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
package telegram
2+
3+
import (
4+
"sync/atomic"
5+
"testing"
6+
)
7+
8+
// TestHandleUpdate_RecoverFromPanic verifies that HandleUpdate catches panics
9+
// in the handler callbacks and continues processing subsequent updates.
10+
func TestHandleUpdate_RecoverFromPanic(t *testing.T) {
11+
ts := testServer(t, nil)
12+
defer ts.Close()
13+
bot := testBot(t, ts)
14+
h := NewHandler(bot)
15+
16+
// Set up a text handler that panics.
17+
var panicCaught atomic.Bool
18+
h.OnTextMessage = func(chatID int64, messageID int, text string) (string, error) {
19+
panic("simulated handler panic")
20+
}
21+
22+
// Set up OnError to verify it fires.
23+
var errorFired atomic.Bool
24+
h.OnError = func(chatID int64, err error) {
25+
errorFired.Store(true)
26+
}
27+
28+
// Wrap HandleUpdate to catch the panic (so the test doesn't crash).
29+
func() {
30+
defer func() {
31+
if r := recover(); r != nil {
32+
panicCaught.Store(true)
33+
// This is what the test verifies — the panic should NOT
34+
// escape HandleUpdate. When the fix is applied, HandleUpdate
35+
// itself catches the panic, so this defer should never fire.
36+
}
37+
}()
38+
h.HandleUpdate(Update{
39+
ID: 1,
40+
Message: &Message{
41+
ID: 42,
42+
Chat: &Chat{ID: 123},
43+
From: &User{ID: 456},
44+
Text: "hello",
45+
},
46+
})
47+
}()
48+
49+
if panicCaught.Load() {
50+
t.Error("HandleUpdate did not recover from panic — panic escaped to caller")
51+
}
52+
53+
if !errorFired.Load() {
54+
t.Error("OnError was not called after panic recovery")
55+
}
56+
}
57+
58+
// TestHandleUpdate_RecoverFromPanicCallback verifies panic recovery
59+
// in callback query handlers.
60+
func TestHandleUpdate_RecoverFromPanicCallback(t *testing.T) {
61+
ts := testServer(t, nil)
62+
defer ts.Close()
63+
bot := testBot(t, ts)
64+
h := NewHandler(bot)
65+
66+
h.OnCallbackQuery = func(chatID int64, data string) (string, error) {
67+
panic("simulated callback panic")
68+
}
69+
70+
var errorFired atomic.Bool
71+
h.OnError = func(chatID int64, err error) {
72+
errorFired.Store(true)
73+
}
74+
75+
var panicEscaped atomic.Bool
76+
func() {
77+
defer func() {
78+
if r := recover(); r != nil {
79+
panicEscaped.Store(true)
80+
}
81+
}()
82+
h.HandleUpdate(Update{
83+
ID: 2,
84+
CallbackQuery: &CallbackQuery{
85+
ID: "cq_test",
86+
From: &User{ID: 456},
87+
Message: &Message{
88+
Chat: &Chat{ID: 789},
89+
},
90+
Data: "test_data",
91+
},
92+
})
93+
}()
94+
95+
if panicEscaped.Load() {
96+
t.Error("HandleUpdate did not recover from callback handler panic")
97+
}
98+
99+
if !errorFired.Load() {
100+
t.Error("OnError was not called after callback panic recovery")
101+
}
102+
}
103+
104+
// TestHandleUpdate_RecoverFromPanicCommand verifies panic recovery
105+
// in command handlers.
106+
func TestHandleUpdate_RecoverFromPanicCommand(t *testing.T) {
107+
ts := testServer(t, nil)
108+
defer ts.Close()
109+
bot := testBot(t, ts)
110+
h := NewHandler(bot)
111+
112+
h.OnCommand = func(chatID int64, messageID int, cmd string, args string) (string, error) {
113+
panic("simulated command panic")
114+
}
115+
116+
var errorFired atomic.Bool
117+
h.OnError = func(chatID int64, err error) {
118+
errorFired.Store(true)
119+
}
120+
121+
var panicEscaped atomic.Bool
122+
func() {
123+
defer func() {
124+
if r := recover(); r != nil {
125+
panicEscaped.Store(true)
126+
}
127+
}()
128+
h.HandleUpdate(Update{
129+
ID: 3,
130+
Message: &Message{
131+
ID: 99,
132+
Chat: &Chat{ID: 111},
133+
From: &User{ID: 222},
134+
Text: "/start",
135+
Entities: []MessageEntity{
136+
{Type: "bot_command", Offset: 0, Length: 6},
137+
},
138+
},
139+
})
140+
}()
141+
142+
if panicEscaped.Load() {
143+
t.Error("HandleUpdate did not recover from command handler panic")
144+
}
145+
146+
if !errorFired.Load() {
147+
t.Error("OnError was not called after command panic recovery")
148+
}
149+
}

0 commit comments

Comments
 (0)