README_CN.md (2)

21-21: Wrap the URL in Markdown to satisfy MD034 and keep punctuation outside the link.
This avoids the “bare URL” lint warning and reads cleaner in rendered Markdown.

-有关代码示例，请参阅 https://github.com/AlexStocks/getty-examples
+有关代码示例，请参阅 [AlexStocks/getty-examples](https://github.com/AlexStocks/getty-examples)。

---

21-21: Convert bare URLs to Markdown links
Replace bare URLs with properly formatted links and ensure trailing punctuation is outside the link:

• README_CN.md:21: change
  有关代码示例，请参阅 https://github.com/AlexStocks/getty-examples
  to
  有关代码示例，请参阅 [getty-examples](https://github.com/AlexStocks/getty-examples)
• README.md:21: change
  For code examples, you can refer to https://github.com/AlexStocks/getty-examples.
  to
  For code examples, you can refer to [getty-examples](https://github.com/AlexStocks/getty-examples).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

transport/callback.go (5)

6-10: Use any instead of interface{} for consistency

The codebase elsewhere uses any; align field types here.

Apply this diff:

 type callbackCommon struct {
-	handler interface{}        // 处理器标识，用于标识回调的来源或类型
-	key     interface{}        // 回调的唯一标识键，与 handler 组合使用
+	handler any                // 处理器标识，用于标识回调的来源或类型
+	key     any                // 回调的唯一标识键，与 handler 组合使用
 	call    func()             // 实际要执行的回调函数
 	next    *callbackCommon    // 指向下一个节点的指针，形成链表结构
 }

---

24-42: Guard against incomparable identifiers to avoid runtime panics on equality

handler/key that are not comparable (e.g., slices, maps) can cause a panic during Remove comparisons. Either document the constraint or proactively reject such additions.

Apply this diff and import reflect:

+import "reflect"
@@
-func (t *callbacks) Add(handler, key interface{}, callback func()) {
+func (t *callbacks) Add(handler, key any, callback func()) {
 	// 防止添加空回调函数
 	if callback == nil {
 		return
 	}
+
+	// handler/key 必须可比较，否则后续移除时可能触发运行时 panic
+	if (handler != nil && !reflect.TypeOf(handler).Comparable()) ||
+		(key != nil && !reflect.TypeOf(key).Comparable()) {
+		return
+	}

And update Remove signature similarly (see next comment).

---

49-73: Keep Remove signature consistent with Add

Update parameter types to any to match Add and the rest of the codebase.

Apply this diff:

-func (t *callbacks) Remove(handler, key interface{}) {
+func (t *callbacks) Remove(handler, key any) {

---

79-86: Harden Invoke: snapshot + per-callback panic isolation

A single callback panic aborts the rest. Also, holding locks during Invoke (if added externally) can deadlock if callbacks call Remove. Execute a snapshot and isolate panics.

Apply this diff:

-func (t *callbacks) Invoke() {
-	// 从头节点开始遍历整个链表
-	for callback := t.first; callback != nil; callback = callback.next {
-		// 确保回调函数不为 nil 再执行
-		if callback.call != nil {
-			callback.call()
-		}
-	}
-}
+func (t *callbacks) Invoke() {
+	// 构建快照，避免遍历期间结构被修改
+	var fns []func()
+	for cb := t.first; cb != nil; cb = cb.next {
+		if cb.call != nil {
+			fns = append(fns, cb.call)
+		}
+	}
+	// 逐个执行，隔离 panic，保证后续回调仍可执行
+	for _, fn := range fns {
+		func(f func()) {
+			defer func() { _ = recover() }()
+			f()
+		}(fn)
+	}
+}

---

1-17: Unify comment language

Repo comments are mostly English; consider translating these headers for consistency.

transport/session.go (1)

879-888: Include stack trace when a close-callback panics

Match existing panic logging style elsewhere in this file for easier debugging.

Apply this diff:

-			go func() {
-				defer func() {
-					if r := recover(); r != nil {
-						log.Errorf("invokeCloseCallbacks panic: %v", r)
-					}
-				}()
-				// 执行关闭回调函数
-				s.invokeCloseCallbacks()
-			}()
+			go func() {
+				defer func() {
+					if r := recover(); r != nil {
+						const size = 64 << 10
+						buf := make([]byte, size)
+						n := runtime.Stack(buf, false)
+						log.Errorf("invokeCloseCallbacks panic: %v\n%s", r, string(buf[:n]))
+					}
+				}()
+				// 执行关闭回调函数
+				s.invokeCloseCallbacks()
+			}()

transport/callback_test.go (1)

1-100: Minor: unify test messages language

Consider English messages for consistency with the rest of the repo.

transport/session_callback_test.go (4)

19-26: Make the test deterministic; replace sleeps/mutex with a done channel.

Sleeping to “wait” for callbacks is flaky and slow. A buffered channel makes the assertion race-free and reliable.

-	var callbackExecuted bool
-	var callbackMutex sync.Mutex
-	
-	callback := func() {
-		callbackMutex.Lock()
-		callbackExecuted = true
-		callbackMutex.Unlock()
-	}
+	doneCh := make(chan struct{}, 1)
+	callback := func() { doneCh <- struct{}{} }
@@
-	// 等待回调执行
-	time.Sleep(50 * time.Millisecond)
-	
-	callbackMutex.Lock()
-	if !callbackExecuted {
-		t.Error("回调函数未被执行")
-	}
-	callbackMutex.Unlock()
+	// 等待回调执行（带超时）
+	select {
+	case <-doneCh:
+	case <-time.After(200 * time.Millisecond):
+		t.Error("回调函数未在预期时间内执行")
+	}

Also applies to: 49-57

---

67-79: Eliminate flakiness in multi-callback test; count via channel instead of sleeps.

Use a buffered channel to collect completions instead of shared counters and sleeps.

-	var callbackCount int
-	var callbackMutex sync.Mutex
+	doneCh := make(chan struct{}, totalCallbacks)
@@
-		callback := func() {
-			callbackMutex.Lock()
-			callbackCount++
-			callbackMutex.Unlock()
-		}
+		callback := func() { doneCh <- struct{}{} }
@@
-	time.Sleep(50 * time.Millisecond)
-	
-	callbackMutex.Lock()
-	if callbackCount != expectedAfterRemove {
-		t.Errorf("期望执行的回调数量为 %d，实际为 %d", expectedAfterRemove, callbackCount)
-	}
-	callbackMutex.Unlock()
+	// 等待剩余回调全部完成（带超时）
+	timeout := time.After(300 * time.Millisecond)
+	for i := 0; i < expectedAfterRemove; i++ {
+		select {
+		case <-doneCh:
+		case <-timeout:
+			t.Fatalf("仅收到 %d/%d 个回调完成信号", i, expectedAfterRemove)
+		}
+	}

Also applies to: 99-105

---

120-130: Avoid shadowing the callbacks type name; rename the local slice for clarity.

The local variable named “callbacks” shadows the type callbacks and hurts readability.

-	// 添加多个不同类型的关闭回调
-	callbacks := []struct {
+	// 添加多个不同类型的关闭回调
+	cases := []struct {
 		handler string
 		key     string
 		action  string
 	}{
@@
-	// 注册所有回调
-	for _, cb := range callbacks {
+	// 注册所有回调
+	for _, cb := range cases {
 		cbCopy := cb // 捕获循环变量
 		callback := func() {
 			callbackMutex.Lock()
 			callbackResults = append(callbackResults, cbCopy.action)
 			callbackMutex.Unlock()
 		}
 		s.AddCloseCallback(cbCopy.handler, cbCopy.key, callback)
 	}

Also update the length check below:

-	expectedCount := len(callbacks)
+	expectedCount := len(cases)

Also applies to: 132-141

---

148-159: Remove arbitrary sleep; assert synchronously or use a wait primitive.

After calling Invoke(), rely on immediate execution or a wait primitive (e.g., WaitGroup/channel) instead of time.Sleep(10ms). This reduces test flakiness.

transport/session_callback.go (2)

3-14: Clarify key constraints in docs.

If the underlying registry keys on handler/key directly, require them to be comparable (or stringify them internally) to avoid runtime panics.

---

61-69: Naming nit: prefer CallbackFunc.

Minor consistency nit with common Go naming (“Callback” vs “CallBack”).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

transport/callback_test.go (4)

8-12: Add a no-op Invoke on empty registry to catch panics early.
Covers the empty-path behavior explicitly.

   if cb.Count() != 0 {
     t.Errorf("Expected count for empty list is 0, but got %d", cb.Count())
   }
+
+  // Ensure invoking on an empty registry is a no-op (no panic).
+  cb.Invoke()

---

33-37: Also assert replacement semantics for duplicate (handler,key).
Ensure Add replaces (not duplicates) when a non-nil fn is added for the same pair.

   cb.Add(remove, remove, nil)
   if cb.Count() != expectedCallbacks {
     t.Errorf("Expected count after adding nil callback is %d, but got %d", expectedCallbacks, cb.Count())
   }
+
+  // Replace an existing callback with a non-nil one; count should remain unchanged.
+  cb.Add(remove, remove, func() { count += remove })
+  if cb.Count() != expectedCallbacks {
+    t.Errorf("Expected count after replacing existing callback is %d, but got %d", expectedCallbacks, cb.Count())
+  }

---

49-52: Rename var for clarity.
This value is a sum, not a count.

- expectedCount := expected - remove
- if count != expectedCount {
-   t.Errorf("Expected execution result is %d, but got %d", expectedCount, count)
+ expectedSum := expected - remove
+ if count != expectedSum {
+   t.Errorf("Expected execution result is %d, but got %d", expectedSum, count)

---

92-92: Add a panic-safety test for Invoke.
If Invoke is intended to be panic-safe, assert it.

 }
+
+func TestCallbackInvokePanicSafe(t *testing.T) {
+	cb := &callbacks{}
+	var ran bool
+	cb.Add("h", "k1", func() { panic("boom") })
+	cb.Add("h", "k2", func() { ran = true })
+	// Expect: Invoke swallows panics and continues executing remaining callbacks.
+	cb.Invoke()
+	if !ran {
+		t.Errorf("Expected subsequent callbacks to run even if one panics")
+	}
+}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

transport/session.go (1)

105-106: API surface expanded—verify all implementers and document contract

• Breaking change: all Session implementers must define these methods.
• Please also document the callback semantics (when invoked relative to OnClose, single vs multiple invocation, thread-safety, idempotency, and whether callbacks may block).

Apply optional inline docs:

-    AddCloseCallback(handler, key any, callback CallBackFunc)
-    RemoveCloseCallback(handler, key any)
+    // Register a function to run when the session closes. Should be fast/non-blocking and idempotent.
+    AddCloseCallback(handler, key any, callback CallBackFunc)
+    // Remove a previously registered close callback identified by (handler, key).
+    RemoveCloseCallback(handler, key any)

transport/session.go (2)

141-145: Callbacks state needs explicit init/Reset intent—confirm zero-value invariants

• Storing closeCallback by value assumes its zero value is a valid empty registry. Confirm this is true.
• Reset() reassigns *s = session{...}, implicitly zeroing both closeCallback and closeCallbackMutex. If that’s intended (drop all callbacks), please make it explicit or add a comment; if not, reinitialize/retain appropriately. Ensure Reset() is never called concurrently.

Optionally make the intent explicit in Reset:

 func (s *session) Reset() {
-    *s = session{
+    *s = session{
         name:   defaultSessionName,
         once:   &sync.Once{},
         done:   make(chan struct{}),
         period: period,
         wait:   pendingDuration,
         attrs:  gxcontext.NewValuesContext(context.Background()),
+        // callbacks: intentionally cleared on reset
+        // closeCallback: zero-value (empty registry)
+        // closeCallbackMutex: zero-value (unlocked)
     }
 }

---

872-880: Safer panic logging for callback goroutine; clarify ordering vs OnClose

• Capture a stack trace to aid debugging if callbacks panic.
• Please also confirm whether callbacks are intended to run before, after, or concurrently with listener.OnClose(s); today they run concurrently. If ordering matters, enforce or document it.

Apply this small logging improvement:

-           go func() {
-               defer func() {
-                   if r := recover(); r != nil {
-                       log.Errorf("invokeCloseCallbacks panic: %v", r)
-                   }
-               }()
-               s.invokeCloseCallbacks()
-           }()
+           go func() {
+               defer func() {
+                   if r := recover(); r != nil {
+                       const size = 64 << 10
+                       buf := make([]byte, size)
+                       buf = buf[:runtime.Stack(buf, false)]
+                       log.Errorf("invokeCloseCallbacks panic: %v\n%s", r, buf)
+                   }
+               }()
+               s.invokeCloseCallbacks()
+           }()

Optional: if an endpoint task pool exists, consider dispatching via EndPoint().GetTaskPool().AddTaskAlways(...) instead of a bare goroutine.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

transport/session_callback.go (3)

31-38: Fix TOCTOU and nil-callback; lock first, re-check closed, ignore nil.

Race between IsClosed and Lock; nil f would panic downstream.

 func (s *session) AddCloseCallback(handler, key any, f CallBackFunc) {
-	if s.IsClosed() {
-		return
-	}
-	s.closeCallbackMutex.Lock()
-	s.closeCallback.Add(handler, key, f)
-	s.closeCallbackMutex.Unlock()
+	if f == nil {
+		return
+	}
+	s.closeCallbackMutex.Lock()
+	defer s.closeCallbackMutex.Unlock()
+	if s.IsClosed() {
+		return
+	}
+	s.closeCallback.Add(handler, key, f)
 }

---

52-59: Apply same TOCTOU fix to RemoveCloseCallback.

Check closed under the lock to avoid races.

 func (s *session) RemoveCloseCallback(handler, key any) {
-	if s.IsClosed() {
-		return
-	}
-	s.closeCallbackMutex.Lock()
-	s.closeCallback.Remove(handler, key)
-	s.closeCallbackMutex.Unlock()
+	s.closeCallbackMutex.Lock()
+	defer s.closeCallbackMutex.Unlock()
+	if s.IsClosed() {
+		return
+	}
+	s.closeCallback.Remove(handler, key)
 }

---

61-76: Don’t hold locks while invoking callbacks; add panic recovery.

Invoking under RLock can deadlock if callbacks call Add/Remove; comment promises recover but none exists.

 func (s *session) invokeCloseCallbacks() {
-	s.closeCallbackMutex.RLock()
-	s.closeCallback.Invoke()
-	s.closeCallbackMutex.RUnlock()
+	// run without holding locks to avoid re-entrancy deadlocks
+	defer func() {
+		if r := recover(); r != nil {
+			// TODO: log recover with stack
+		}
+	}()
+	s.closeCallback.Invoke()
 }

transport/session_callback_test.go (5)

60-74: Deflake: wait on a signal instead of sleeping.

Sleep-based waiting is brittle and slow; signal when callback runs.

-		go func() {
-			time.Sleep(10 * time.Millisecond)
-			s.stop()
-		}()
-
-		// Wait for callback execution
-		time.Sleep(50 * time.Millisecond)
+		doneCh := make(chan struct{}, 1)
+		// augment callback to signal completion
+		orig := callback
+		callback = func() {
+			orig()
+			doneCh <- struct{}{}
+		}
+		// re-register with signal-capable callback
+		s.RemoveCloseCallback("testHandler", "testKey")
+		s.AddCloseCallback("testHandler", "testKey", callback)
+
+		go s.stop()
+		select {
+		case <-doneCh:
+		case <-time.After(200 * time.Millisecond):
+			t.Fatal("timeout waiting for callback")
+		}

---

110-123: Deflake multi-callback assertion; count signals instead of sleeping.

Use a buffered channel to await exact number of executions.

-		// Test execution of remaining callbacks when closing
-		go func() {
-			time.Sleep(10 * time.Millisecond)
-			s.stop()
-		}()
-
-		time.Sleep(50 * time.Millisecond)
+		// Test execution of remaining callbacks when closing
+		doneCh := make(chan struct{}, totalCallbacks)
+		// re-register callbacks to signal execution
+		s.closeCallback = callbacks{}
+		for i := 1; i < totalCallbacks; i++ { // skip 0 as removed
+			idx := i
+			s.AddCloseCallback(fmt.Sprintf("handler%d", idx), fmt.Sprintf("key%d", idx), func() {
+				callbackMutex.Lock()
+				callbackCount++
+				callbackMutex.Unlock()
+				doneCh <- struct{}{}
+			})
+		}
+		go s.stop()
+		for i := 0; i < expectedAfterRemove; i++ {
+			select {
+			case <-doneCh:
+			case <-time.After(200 * time.Millisecond):
+				t.Fatalf("timeout waiting for callback %d", i)
+			}
+		}

---

170-176: Exercise the production path: call invokeCloseCallbacks() directly; drop sleeps.

This ensures recover/lock semantics are actually tested.

-		// Execute all close callbacks
-		s.closeCallback.Invoke()
-
-		// Wait for callback execution to complete
-		time.Sleep(10 * time.Millisecond)
+		// Execute all close callbacks via session
+		s.invokeCloseCallbacks()
@@
-		// Execute callbacks again
-		s.closeCallback.Invoke()
-		time.Sleep(10 * time.Millisecond)
+		// Execute callbacks again
+		s.invokeCloseCallbacks()

Also applies to: 198-201

---

225-234: Consistency: prefer session.invokeCloseCallbacks in edge-case checks.

Keeps tests aligned with real invocation behavior.

-		// Execute empty callback list (should not panic)
-		s.closeCallback.Invoke()
+		// Execute empty callback list (should not panic)
+		s.invokeCloseCallbacks()
@@
-		// Execute empty list after removal (should not panic)
-		s.closeCallback.Invoke()
+		// Execute empty list after removal (should not panic)
+		s.invokeCloseCallbacks()

---

27-35: Add tests for nil callbacks, add-after-close, and panic recovery.

Covers important edge cases implied by the API and comments.

I can add subtests:

• AddCloseCallback(nil) is ignored.
• AddCloseCallback after s.stop() is ignored.
• A callback that panics does not crash tests (recover verified).
  Want me to push a patch?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

README_CN.md (1)

34-47: Add a short runtime guidance note.

建议在示例下补一句“关闭回调应尽量快速、避免阻塞或长耗时操作，否则可能延长会话关闭耗时；重活移交到独立 goroutine”。

transport/callback.go (1)

118-129: Optional: track size for O(1) Len.

若 Len 调用较频繁，可在结构体中维护 size 计数，在 Add/Remove 中增减，避免每次 O(n) 遍历。当前实现功能正确，此项为优化建议。

README.md (1)

34-47: State runtime expectations for callbacks.

Add a one-liner: “Callbacks should be fast/non-blocking; move heavy work to separate goroutines to avoid delaying shutdown.”

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

README_CN.md (2)

31-33: 文档与实现不一致：Panic 语义需与代码/测试统一。

当前文档称“panic 会被正确处理并记录堆栈跟踪”，但此前实现与测试为“panic 直接冒泡”。请二选一并在文档中明确。建议先按现状修正文案（若未来实现 recover 再回改）。

建议补丁（若保持“冒泡”语义）：

- - **Panic 安全性**：回调中的 panic 会被正确处理并记录堆栈跟踪
+ - **Panic 语义**：回调中的 panic 将冒泡（由调用方处理）。如在 Session 层做 recover/记录，请在此注明具体位置与行为。

可运行脚本快速验证当前仓库的实际语义（是否在 callbacks.Invoke/Session 中有 recover）：

#!/bin/bash
rg -nP 'func\s+(\([^)]*\)\s*)?Invoke\s*\(' -C3 --type go
rg -n 'recover\(' -C2 --type go
rg -n '\.Invoke\s*\(' -C2 --type go

---

53-56: 补充“键类型可比较性（comparable）”约束，避免运行时 panic。

实现使用“==”比较 handler/key，需提醒用户不要传入 map/slice/function 等不可比较类型。

建议在此小节追加一条要点：

 - **RemoveCloseCallback**：移除之前注册的回调
 - **线程安全**：所有操作都是线程安全的，可以并发调用
+ - **键类型约束**：handler 和 key 必须是可比较（comparable）类型（如 string、int、指针）。请勿使用 map/slice/function。
+   如需复杂标识，建议使用稳定字符串或结构体指针作为标识。

README_CN.md (1)

23-56: 进一步澄清并发语义与 API 细节，减少歧义。

• 说明“执行期间的并发修改语义”：在一次关闭触发的执行中，对回调表的新增/移除是否影响“本次执行”（快照 vs. 实时）。
• 补充 API 签名片段，便于读者检索与 IDE 跳转：AddCloseCallback(handler, key any, cb func())、RemoveCloseCallback(handler, key any)。

可在“回调管理”末尾追加：

+ - **执行期间的一致性**：回调执行过程中对注册表的新增/移除对“本次执行”的影响语义为：<填写“按快照执行/实时可见”之一>。
+
+ 签名参考：
+ ```go
+ type CallBackFunc func()
+ func (s *Session) AddCloseCallback(handler, key any, cb CallBackFunc)
+ func (s *Session) RemoveCloseCallback(handler, key any)
+ ```

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

README_CN.md (1)

29-33: Doc vs code: “Panic 安全性”与实现/测试不一致；请二选一并统一。
当前文档宣称会捕获/记录 panic（Line 31），但之前代码与测试表现为让 panic 冒泡。请按选型同步文档或代码；给出就地修订示例（若保持冒泡语义）：

- - **Panic 安全性**：回调中的 panic 会被正确处理并记录堆栈跟踪
+ - **Panic 语义**：回调中的 panic 将冒泡，由调用方负责处理（例如在 Session 层或上层统一 recover）

若要改为“每个回调独立 recover 并记录堆栈，同时不中断后续回调执行”，则需实现层面更新 callbacks.Invoke 与相应测试，并将此处描述改为“对每个回调做 recover 并记录堆栈，不中断后续回调”。

README_CN.md (5)

32-33: 明确执行顺序是 FIFO 还是 LIFO。
“有序执行”过于笼统，建议明确具体顺序，避免误解：

- - **有序执行**：回调按照添加的顺序执行
+ - **执行顺序**：回调按照添加的顺序执行（FIFO）

如实现为栈式（头插），则应标注为 LIFO。

---

49-56: 说明回调是在关闭路径中的同步/异步执行方式。
为便于使用者评估关闭时延与阻塞影响，请注明 Close 回调执行模型（同步于关闭流程/异步 goroutine、是否并行执行、是否有超时/截止时间）。

可追加一句：“回调在会话关闭时于同一关闭流程中同步执行，建议快速返回；若需耗时处理，请另启 goroutine。”

---

53-55: 补充方法签名，便于读者快速对齐用法。

- - **AddCloseCallback**：注册一个在会话关闭时执行的回调
- - **RemoveCloseCallback**：移除之前注册的回调
+ - **AddCloseCallback**：注册一个在会话关闭时执行的回调
+ - **RemoveCloseCallback**：移除之前注册的回调
+
+```go
+// 回调函数类型
+type CloseCallback func()
+
+// 方法签名（示例）
+AddCloseCallback(handler any, key any, cb CloseCallback)
+RemoveCloseCallback(handler any, key any) bool
+```

---

25-26: 范围措辞更精确：当前仅提供“关闭回调”。
避免让读者误解已支持更多生命周期事件，建议加“目前提供关闭回调（CloseCallback）”字样。

- Getty 提供了一个强大的回调系统，允许您为会话生命周期事件注册和管理回调函数。
+ Getty 提供了一个强大的回调系统，允许您为会话生命周期事件注册和管理回调函数；当前提供“关闭回调（CloseCallback）”。

---

87-93: 给出具体 API 名称与简短重试示例，提升可操作性。
建议点名常用发送 API 名称，并示例如何依据错误做上层重试/放弃：

- 上层调用者需要根据这些结果决定是否加入重试机制。
+ 上层调用者需要根据这些结果决定是否加入重试机制（例如基于 Session.WritePkg/Send/WriteBytes 的返回错误进行判断）。
@@
- 总之，Getty 的数据传输接口并不自带内部的重试机制；相反，是否在特定情况下实现重试逻辑由上层调用者决定。
+ 示例：
+ ```go
+ if err := sess.WritePkg(msg, timeout); err != nil {
+     if errors.Is(err, syscall.EAGAIN) { // 发送缓冲区繁忙
+         // 上层自行等待/退避后重试
+     } else {
+         // 由自动重连/业务容错决定是否重试或丢弃
+     }
+ }
+ ```
+ 总之，Getty 的数据传输接口不自带内部重试；是否以及如何重试由上层决定。

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

README.md (2)

57-85: Great: clear comparability guidance addresses earlier feedback.

The supported/unsupported type lists and examples are accurate and helpful.

---

29-33: Docs claim panic safety; confirm implementation or adjust text.

README says panics are recovered and logged, but prior code/tests reportedly propagate panics. Align behavior or documentation.

Two alternative patches—pick the one that matches actual behavior:

Option A (recover-and-log is implemented):

- - **Panic safety**: Callback panics are properly handled and logged with stack traces
+ - **Panic safety**: Callback panics are recovered and logged with stack traces; panics do not escape invocation

Option B (panics currently propagate):

- - **Panic safety**: Callback panics are properly handled and logged with stack traces
+ - **Panic behavior**: Callback panics propagate to the caller; use wrapper callbacks if you need recovery/logging

If A: briefly document logger used and log level.

README.md (4)

29-31: Clarify replace semantics vs execution order.

When replacing an existing (handler,key), does the callback keep its original position or move to the tail? Callers may rely on order guarantees.

---

34-47: State the invocation model (serial vs parallel).

Examples imply ordered execution. Explicitly say callbacks run serially in registration order and on which goroutine, to set latency expectations.

Apply:

 // Callbacks are automatically executed when the session closes
+
+// Notes:
+// - Callbacks run serially in registration order on a dedicated goroutine during shutdown.
+// - A slow callback delays subsequent callbacks; offload heavy work to your own goroutines.

---

49-49: Expand the non-blocking guidance with a one-liner.

Mention that slow callbacks delay later ones if execution is serial.

-**Note**: Callbacks should be fast/non-blocking; move heavy work to separate goroutines to avoid delaying shutdown.
+**Note**: Callbacks should be fast/non-blocking; because they execute in order, a slow callback delays later ones. Offload heavy work to separate goroutines.

---

51-56: Document idempotency and reentrancy constraints.

• Is RemoveCloseCallback a no-op if the pair doesn’t exist?
• Are Add/Remove allowed during Invoke? If not, call it out to avoid deadlocks.

 - **RemoveCloseCallback**: Remove a previously registered callback
 - **Thread Safety**: All operations are thread-safe and can be called concurrently
+- **Idempotency**: RemoveCloseCallback is a no-op if the (handler, key) pair does not exist
+- **During invocation**: Add/Remove are not applied to the in-flight invocation pass; they take effect on subsequent invocations (recommended)

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

README.md (1)

29-33: Panic-safety claim: align docs with behavior.

Docs say panics are recovered and logged. Verify implementation/tests do this; otherwise adjust wording.

If code recovers panics, consider this doc tweak for precision:

- **Panic safety**: Callback panics are properly handled and logged with stack traces
+ **Panic safety**: Each callback runs with panic recovery; panics are recovered, logged (with stack traces), and do not stop subsequent callbacks

If panics currently propagate, change to:

- **Panic safety**: Callback panics are properly handled and logged with stack traces
+ **Panic behavior**: Panics propagate to the caller; use application-level recovery if needed

README.md (4)

49-50: Clarify order vs goroutine semantics.

To avoid ambiguity, state that callbacks run sequentially within a single goroutine to preserve ordering.

- **Note**: Callbacks are executed in a separate goroutine during session shutdown.
+ **Note**: During session shutdown, callbacks are executed sequentially in a dedicated goroutine to preserve the add-order.

---

53-56: Document Remove semantics and idempotency.

Spell out behavior when the key isn’t found and whether Remove is idempotent/what it returns (if anything).

- **RemoveCloseCallback**: Remove a previously registered callback
+ **RemoveCloseCallback**: Remove a previously registered callback (no-op if not found; safe to call multiple times)

---

29-31: “Replace semantics”: specify position after replacement.

Does replacement keep the original position or move the entry to the tail? Make it explicit to prevent surprises.

- **Replace semantics**: Adding a callback with the same handler and key will replace the existing one
+ **Replace semantics**: Adding with the same (handler, key) replaces the existing callback in place (position preserved). 
+ If your implementation moves it to the end, document that instead.

---

61-68: Caveat for float/complex keys.

These are comparable but error‑prone (NaN != NaN; rounding). Recommend discouraging them as identifiers.

- **Basic types**: `string`, `int`, ... `float32`, `float64`, `bool`, `complex64`, `complex128`
+ **Basic types**: `string`, `int`, ... `float32`, `float64`, `bool`, `complex64`, `complex128`
+   - ⚠️ Avoid `float*`/`complex*` as keys due to NaN and precision semantics; prefer strings/ints.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

README.md (5)

29-33: Clarify where panic recovery happens to avoid overpromising.

Document that recovery/logging is performed by the session close path (goroutine with defer/recover), not by a generic Invoke. This keeps README consistent with implementation and tests.

Apply this wording tweak:

- - **Panic safety**: Callback panics are properly handled and logged with stack traces
+ - **Panic safety**: During session close, callbacks run in a dedicated goroutine with defer/recover; panics are logged with stack traces and do not escape the close path

---

49-50: Strengthen the note with explicit panic‑recovery behavior.

This removes ambiguity about where ordering and recovery are enforced.

-**Note**: During session shutdown, callbacks are executed sequentially in a dedicated goroutine to preserve the add-order.
+**Note**: During session shutdown, callbacks are executed sequentially in a dedicated goroutine to preserve add‑order, with defer/recover to log panics without letting them escape the close path.

---

36-47: Minor example polish: show that removal is optional and idempotent.

Add a short comment to signal it’s safe to call Remove even if not present.

 // Remove a specific callback
-session.RemoveCloseCallback("cleanup", "resources")
+// Safe to call even if the pair was never added (no-op)
+session.RemoveCloseCallback("cleanup", "resources")

---

23-24: PR hygiene: consider updating the PR title to reflect both link fixes and close‑callback docs.

E.g., “Docs: fix example links and add per‑session close‑callback docs (EN/ZN)”.

---

57-86: README: "non‑comparable are logged and ignored" is inaccurate — comparability is checked but no log is emitted.

• Verified: comparability is enforced in transport/callback.go (uses reflect.TypeOf(v).Comparable()); AddCloseCallback delegates to s.closeCallback.Add(...).
• Missing: I found no logging at the rejection path — non‑comparable values are detected and skipped but not logged; callbacks use "==" for lookup (callback.go).
• Recommendation: either remove the "logged" claim and replace the exhaustive type list with a concise rule ("values must be comparable — see Go spec for comparison rules"), or add an explicit log at the rejection point (transport/callback.go).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: ayamzh
PR: AlexStocks/getty#91
File: README_CN.md:0-0
Timestamp: 2025-09-18T08:16:03.103Z
Learning: In Getty's callback system, while callbacks.Invoke() propagates panics, the session layer in transport/session.go wraps the callback invocation in a goroutine with defer/recover to handle panics safely and log them with stack traces, making the "Panic 安全性" documentation in README accurate.

Learnt from: ayamzh
PR: AlexStocks/getty#91
File: README_CN.md:0-0
Timestamp: 2025-09-18T08:16:03.103Z
Learning: In Getty's callback system, while callbacks.Invoke() propagates panics, the session layer in transport/session.go wraps the callback invocation in a goroutine with defer/recover to handle panics safely and log them with stack traces, making the "Panic 安全性" documentation in README accurate.

Learnt from: ayamzh
PR: AlexStocks/getty#91
File: README_CN.md:0-0
Timestamp: 2025-09-18T08:16:03.103Z
Learning: In Getty's callback system, while callbacks.Invoke() propagates panics, the session layer in transport/session.go wraps the callback invocation in a goroutine with defer/recover to handle panics safely and log them with stack traces, making the "Panic 安全性" documentation in README accurate.