Fix/node sse client

[greepar]

修复支持路径前缀 , SSE连接错误

Summary by CodeRabbit

• Bug Fixes

  • More reliable event streaming with improved connection handling and recovery.
  • Corrected URL resolution to ensure stable connections across configurations.

• Enhancements

  • Connect command now parses parameters more robustly (trimmed and case-normalized) for fewer input errors.

• Chores

  • Removed an unused SSE runtime dependency.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Replaced fetch-based SSE handling with the eventsource library's EventSource client; adjusted message handling, lifecycle (open/error/reconnect) and cleanup. Also changed URL base construction and bot Client initialization to use normalized base URLs and updated a command's parameter parsing.

Changes

Cohort / File(s)	Summary
SSE Connection Implementation src/connection/ServerSentEvents.ts	Replaced fetchEventSource + AbortController with eventsource's EventSource stored in #client; injects authorization via custom fetch; registers milky_event listener that JSON-parses messages and dispatches via EventDispatch; lifecycle moved to #client.onopen / #client.onerror; reconnection/cleanup now calls #client.close() and nulls client.
Core URL & Client Init src/core/Client.ts, src/core/bot.ts	Changed new URL('/api', url) → new URL('api', ...) with explicit trailing-slash normalization; MilkyAdapter.adapter.address built with relative event path and normalized base; Client now initialized with raw cfg.url instead of derived adapter.address; error creation in init now includes info as second argument.
Bot Command Parsing src/apps/bot.ts	Connect command parsing switched from regex to ctx.msg.substring(8).trim() then split by ,; protocol/address trimmed and lowercased before lookup.
Dependencies package.json	Removed @microsoft/fetch-event-source dependency (runtime removal).

Sequence Diagram(s)

sequenceDiagram
  participant Bot
  participant Client
  participant EventSource as SSEClient
  participant Server
  participant Dispatcher as EventDispatch

  Bot->>Client: start/connect(cfg.url)
  Client->>SSEClient: new EventSource(url, {fetch: with Authorization})
  SSEClient-->>Client: onopen
  Client->>Client: init connectTime, reset reconnectCount, register bot
  SSEClient-->>Client: milky_event (data)
  Client->>Dispatcher: parse JSON, dispatch event
  note right of Dispatcher: Dispatcher handles incoming events
  SSEClient-->>Client: onerror
  Client->>Client: log error, maybe unregister bot
  Client->>Client: reconnect() -> close old SSEClient, create new EventSource
  SSEClient->>Server: HTTP SSE stream

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped on streams where events fly,

EventSource opened wide its eye,
URLs trimmed tidy, headers set true,
Messages parsed, dispatched anew,
A happy rabbit hops—code stitched through.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix/node sse client' is vague and generic, using non-descriptive formatting that doesn't clearly convey what was actually fixed or changed in the substantial modifications across multiple files.	Consider a more specific title that describes the main change, such as 'Migrate SSE transport from fetchEventSource to EventSource library' or similar, which would better reflect the primary refactoring work performed.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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 and usage tips.}

[github-actions]

🙌 你好，贡献者！感谢你提交的第一个拉取请求！🛠️ 你正在帮助我们打造更好的项目，我们会认真审查并与你一起优化它。💡 如果你有任何问题或需要帮助，随时联系！✨ 很高兴与你合作，欢迎成为我们社区的一员！

[gemini-code-assist]

Code Review

This pull request migrates the SSE connection logic from @microsoft/fetch-event-source to the eventsource library and updates URL construction to handle trailing slashes more consistently. Review feedback identifies a critical issue where the eventsource constructor is incorrectly passed a fetch option, and suggests improvements for error handling during JSON parsing and more effective logging of connection errors.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/connection/ServerSentEvents.ts (1)

84-91: ⚠️ Potential issue | 🟠 Major

Cancel pending reconnect backoff in clear().

stop() calls clear(), but #reconnectTimer survives. If a disconnect had already scheduled a retry, the timeout still fires after shutdown and creates a new EventSource on a bot that was meant to stay offline.

Suggested fix

   clear () {
+    if (this.#reconnectTimer) {
+      clearTimeout(this.#reconnectTimer)
+      this.#reconnectTimer = null
+    }
     if (this.#IntervalTime) {
       clearInterval(this.#IntervalTime)
       this.#IntervalTime = null
     }
     if (this.#client) {
       this.#client.close()
       this.#client = null
     }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/connection/ServerSentEvents.ts` around lines 84 - 91, The clear() method
currently cancels the interval and closes the client but does not cancel a
pending reconnect, so add logic in clear() to clear and nullify the reconnect
timeout (this.#reconnectTimer) to prevent a scheduled reconnect from creating a
new EventSource after stop() is called; ensure that whichever method schedules
the reconnect uses the same this.#reconnectTimer field so clearing it here
prevents the retry from firing.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/connection/ServerSentEvents.ts`:
- Around line 34-36: The listener registered in
this.#client.addEventListener('milky_event', ...) currently calls
JSON.parse(event.data) and EventDispatch(data, this.bot) directly, so any thrown
error will escape the handler and bypass reconnect/error handling; wrap the
parse+dispatch in a try/catch inside that listener, catch any parsing or
dispatch errors, log the error (including the raw event.data or a safe summary)
using your existing logger (e.g., this.bot or process logger) and return/ignore
the bad frame so the transport stays alive; ensure you reference
JSON.parse(event.data) and EventDispatch(...) when adding the try/catch so only
this frame is suppressed and normal processing continues.

In `@src/core/bot.ts`:
- Around line 29-33: Normalize cfg.url once before building derived endpoints so
you don't append /event or /api twice: add a helper (e.g., normalizeBaseUrl)
that parses cfg.url, rejects URLs already pointing to /api or /event, ensures a
trailing slash, and returns the normalized base URL string; then use that
normalized value when constructing the event address (new URL('event',
normalizedBase).toString()) and when instantiating Client (new
Client(normalizedBase, this)) so both the event and API clients reuse the same
validated base URL.

---

Outside diff comments:
In `@src/connection/ServerSentEvents.ts`:
- Around line 84-91: The clear() method currently cancels the interval and
closes the client but does not cancel a pending reconnect, so add logic in
clear() to clear and nullify the reconnect timeout (this.#reconnectTimer) to
prevent a scheduled reconnect from creating a new EventSource after stop() is
called; ensure that whichever method schedules the reconnect uses the same
this.#reconnectTimer field so clearing it here prevents the retry from firing.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 15c7d400-fd09-43ab-963a-4e3eea1a133a

📥 Commits

Reviewing files that changed from the base of the PR and between a5a1afb and 94f023b.

📒 Files selected for processing (3)

• src/connection/ServerSentEvents.ts
• src/core/Client.ts
• src/core/bot.ts