fix: avoid stale stream writes after client disconnect

[seefs001]

⚠️ 提交说明 / PR Notice

Important

• 请提供人工撰写的简洁摘要，避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)

一个请求发出后，等相关 goroutine 都退出后再让 Gin 回收这个 ctx。
用户断开后，把 EndReason 记成 client_gone。

🚀 变更类型 / Type of change

• 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue，避免将设计取舍、理解偏差或预期不一致直接归类为 bug
• ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
• ⚡ 性能优化 / 重构 (Refactor)
• 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

• Closes 流式转发 goroutine 异常退出不完整时可能复用 gin.Context 写入后续请求，存在串包风险 #5707

✅ 提交前检查项 / Checklist

• 人工确认: 我已亲自整理并撰写此描述，没有直接粘贴未经处理的 AI 输出。
• 非重复提交: 我已搜索现有的 Issues 与 PRs，确认不是重复提交。
• Bug fix 说明: 若此 PR 标记为 Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
• 变更理解: 我已理解这些更改的工作原理及可能影响。
• 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
• 本地验证: 已在本地运行并通过测试或手动验证，维护者可以据此复核结果。
• 安全合规: 代码中无敏感凭据，且符合项目代码规范。

📸 运行证明 / Proof of Work

(请在此粘贴截图、关键日志或测试报告，以证明变更生效)

Summary by CodeRabbit

• Bug Fixes
  • Improved streaming behavior for request cancellation/client disconnects by stopping further rendering/flush when the request context is done.
  • Enhanced streaming shutdown reliability with deterministic keep-alive termination, safer write synchronization, and clearer end-reason handling.
  • Standardized OpenAI image streaming SSE chunk formatting for consistent output.
• Tests
  • Added a client-cancel streaming test to confirm prompt handler exit, upstream abort behavior, delivered chunk count, and correct stream end status.

[coderabbitai]

Walkthrough

Adds request-context guards to SSE writes, rewrites stream-scanner shutdown and ping cleanup to wait for goroutines, routes image streaming through shared helpers, and adds a cancellation test for streamed responses.

Changes

Streaming goroutine lifecycle fix

Layer / File(s)	Summary
Request-context write guards relay/helper/common.go	Adds requestContextDone(c *gin.Context) and applies it to FlushWriter, ClaudeData, ClaudeChunkData, ResponseChunkData, StringData, and PingData so canceled requests skip rendering or return nil / a request-context error before writing.
StreamScannerHandler shutdown and wait loop relay/helper/stream_scanner.go	Introduces the handler cancellation path, write-deadline extension, coordinated goroutine shutdown, ping and scanner panic handling, removal of the direct request-context scan branch, and the client-gone end-of-stream cleanup path.
Ping keepalive completion cleanup relay/channel/api_request.go	Changes ping keepalive setup to return a completion channel, simplifies ping sending to a direct locked helper call, and waits for the ping goroutine to exit before request cleanup finishes.
OpenAI image stream helper adoption relay/channel/openai/helper.go, relay/channel/openai/relay_image.go	Routes OpenAI image stream chunk, payload, and done output through shared SSE helpers instead of manually writing event and data frames.
Client cancellation streaming test relay/helper/stream_scanner_test.go	Adds a cancellable streaming test that feeds SSE data through an io.Pipe, cancels mid-stream, and checks processed chunks, end reason, and recorded output.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• QuantumNous/new-api#4608: Shares the same SSE helper path used by the OpenAI image streaming changes here.

Suggested reviewers: Calcium-Ion, creamlike1024

Poem

🐰 I hop through streams with tidy feet,
No stale old writes can claim a seat.
When context fades, the pings grow still,
And goroutines obey the will.
Fresh chunks flow on, then safely rest—
Clean little hops are the best!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The lifecycle fix is mostly there, but request-aware helpers now return nil on done contexts, contrary to the required error signaling after disconnect.	Restore error returns for FlushWriter, StringData, and PingData on done contexts, and keep propagating write failures from ResponseChunkData.
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main fix: preventing stale stream writes after client disconnect.
Out of Scope Changes check	✅ Passed	The changes stay focused on streaming lifecycle, cancellation, write deadlines, and related tests.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@relay/helper/stream_scanner.go`:
- Around line 82-95: The stream scanner cleanup in stream_scanner.go is
happening too late because the deferred block still allows the final status log
to run while goroutines may be active. Move the cancel, response body close,
ticker stops, and wg.Wait logic into a cleanup function protected by sync.Once,
then invoke that cleanup at the done: path before the logging that reads c and
info.ReceivedResponseCount, while keeping defer cleanup() as a fallback safety
net. Use the existing stream-scanner flow around the main scan loop and final
status logging to place this change.
- Around line 124-126: The PingData(c) call inside stream_scanner.go is holding
writeMutex around an unbounded write/flush path, which can block cleanup if the
client is slow or half-closed. Update the ping write in the scanner loop to use
a bounded write path, such as setting a write deadline on the response writer
before calling PingData, and keep the lock held only as long as needed around
the actual write. Use the existing writeMutex and PingData(c) location to make
the change without altering the surrounding cleanup flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f87ca34f-26ff-4535-87bb-9e2e409f0fd4

📥 Commits

Reviewing files that changed from the base of the PR and between 64eafc9 and 37b4977.

📒 Files selected for processing (3)

• relay/helper/common.go
• relay/helper/stream_scanner.go
• relay/helper/stream_scanner_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@relay/channel/api_request.go`:
- Around line 457-468: The synchronous helper.PingData(c) call in sendPingData
can block forever on c.Writer write/flush and prevent stopPinger() from being
observed, so update the ping path to be cancellation-aware or time-bounded
before waiting on the mutex/deferred pingerDone flow. Use the sendPingData
function and the helper.PingData invocation to locate the change, and ensure
stalled writes return or unblock on context cancellation so the handler cannot
hang indefinitely.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48a520a5-72d2-42a6-8c33-421f2884987a

📥 Commits

Reviewing files that changed from the base of the PR and between 37b4977 and 871d4c7.

📒 Files selected for processing (2)

• relay/channel/api_request.go
• relay/helper/stream_scanner.go

🚧 Files skipped from review as they are similar to previous changes (1)

• relay/helper/stream_scanner.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@relay/channel/openai/relay_image.go`:
- Around line 272-279: The SSE write path in writeOpenaiImageStreamData and
writeOpenaiImageStreamDone can silently ignore client disconnects because
helper.ResponseChunkData has no error return and helper.StringData returns nil
on a canceled context. Update these helpers to detect when the request context
is already done and propagate that cancellation back to the caller instead of
treating it as a successful write, so the stream can be ended as client_gone
rather than StreamEndReasonDone.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bd239bf7-818c-4c6b-99cc-abbef10b2b64

📥 Commits

Reviewing files that changed from the base of the PR and between 871d4c7 and 9e8d4d0.

📒 Files selected for processing (2)

• relay/channel/openai/relay_image.go
• relay/helper/stream_scanner.go

🚧 Files skipped from review as they are similar to previous changes (1)

• relay/helper/stream_scanner.go

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Return cancellation errors after guarded SSE writes.

ResponseChunkData has no error return, and StringData returns nil when the request context is already done. That means this path can silently skip payload or [DONE] writes after client disconnect, then the caller records StreamEndReasonDone instead of client_gone.

Proposed fix

 	if eventName != "" {
+		if err := openaiImageRequestContextErr(c); err != nil {
+			return err
+		}
 		helper.ResponseChunkData(c, dto.ResponsesStreamResponse{Type: eventName}, string(data))
-		return nil
+		return openaiImageRequestContextErr(c)
 	}
-	return helper.StringData(c, string(data))
+	if err := helper.StringData(c, string(data)); err != nil {
+		return err
+	}
+	return openaiImageRequestContextErr(c)
 }
 
 func writeOpenaiImageStreamDone(c *gin.Context) error {
-	return helper.StringData(c, "[DONE]")
+	if err := helper.StringData(c, "[DONE]"); err != nil {
+		return err
+	}
+	return openaiImageRequestContextErr(c)
+}
+
+func openaiImageRequestContextErr(c *gin.Context) error {
+	if c == nil || c.Request == nil {
+		return nil
+	}
+	return c.Request.Context().Err()
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	helper.ResponseChunkData(c, dto.ResponsesStreamResponse{Type: eventName}, string(data))
	return nil
	}
	return helper.FlushWriter(c)
	return helper.StringData(c, string(data))
	}
	func writeOpenaiImageStreamDone(c *gin.Context) error {
	if _, err := fmt.Fprint(c.Writer, "data: [DONE]\n\n"); err != nil {
	return err
	}
	return helper.FlushWriter(c)
	return helper.StringData(c, "[DONE]")
	helper.ResponseChunkData(c, dto.ResponsesStreamResponse{Type: eventName}, string(data))
	return openaiImageRequestContextErr(c)
	}
	if err := helper.StringData(c, string(data)); err != nil {
	return err
	}
	return openaiImageRequestContextErr(c)
	}
	func writeOpenaiImageStreamDone(c *gin.Context) error {
	if err := helper.StringData(c, "[DONE]"); err != nil {
	return err
	}
	return openaiImageRequestContextErr(c)
	}
	func openaiImageRequestContextErr(c *gin.Context) error {
	if c == nil || c.Request == nil {
	return nil
	}
	return c.Request.Context().Err()
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@relay/channel/openai/relay_image.go` around lines 272 - 279, The SSE write
path in writeOpenaiImageStreamData and writeOpenaiImageStreamDone can silently
ignore client disconnects because helper.ResponseChunkData has no error return
and helper.StringData returns nil on a canceled context. Update these helpers to
detect when the request context is already done and propagate that cancellation
back to the caller instead of treating it as a successful write, so the stream
can be ended as client_gone rather than StreamEndReasonDone.

[coderabbitai]

🧹 Nitpick comments (1)

relay/channel/openai/helper.go (1)

205-210: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Swallowed write error diverges from the "propagate write errors" objective.

The PR aims to propagate ResponseChunkData write errors so streaming paths stop consuming upstream after client disconnect, yet sendResponsesStreamData discards it via _. Since this helper is void, a write failure (e.g., request context done) won't stop the enclosing loop here. Consider returning the error to the caller so this path can abort like relay_image.go (return helper.ResponseChunkData(...)).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@relay/channel/openai/helper.go` around lines 205 - 210, The
sendResponsesStreamData helper is swallowing ResponseChunkData write failures by
assigning the result to _, which prevents streaming callers from stopping on
client disconnect or other write errors. Update sendResponsesStreamData to
return the error from helper.ResponseChunkData and have the caller in the
streaming flow check and propagate it, matching the pattern used in
relay_image.go so the loop can abort immediately on write failure.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@relay/channel/openai/helper.go`:
- Around line 205-210: The sendResponsesStreamData helper is swallowing
ResponseChunkData write failures by assigning the result to _, which prevents
streaming callers from stopping on client disconnect or other write errors.
Update sendResponsesStreamData to return the error from helper.ResponseChunkData
and have the caller in the streaming flow check and propagate it, matching the
pattern used in relay_image.go so the loop can abort immediately on write
failure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b30baa9c-681c-4c89-96e5-81d1c0a16eb4

📥 Commits

Reviewing files that changed from the base of the PR and between 9e8d4d0 and 48a4986.

📒 Files selected for processing (6)

• relay/channel/api_request.go
• relay/channel/openai/helper.go
• relay/channel/openai/relay_image.go
• relay/helper/common.go
• relay/helper/stream_scanner.go
• relay/helper/stream_scanner_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• relay/channel/openai/relay_image.go
• relay/helper/common.go
• relay/channel/api_request.go