update #187
update #187
Conversation
在SSE会话中存储路由参数，并添加相关函数用于从上下文中获取路由参数。新增WithSSEPattern选项以支持带参数的SSE端点模式。这些更改使得SSE服务器能够处理包含动态路径参数的请求。
- 调整了SSEServer结构体字段的顺序，提高代码可读性 - 优化了ServeHTTP方法中的路径匹配逻辑，使其更加清晰 - 移除了冗余的上下文设置代码 - 添加了必要的空行以提升代码可读性
添加了一个新的示例代码文件 `main.go`，展示了如何使用自定义的SSE（Server-Sent Events）模式。该示例包括创建MCP服务器、注册工具、以及使用自定义路由模式和上下文函数启动SSE服务器
WalkthroughThis change introduces support for dynamic route parameter extraction in the Server-Sent Events (SSE) server implementation. It adds the ability to define SSE endpoint patterns with path parameters (such as /:channel/sse), parse those parameters from incoming request URLs, and inject them into the request context for use during SSE session handling and message processing. A new example demonstrates the usage of a custom SSE pattern and context function that logs and utilizes these route parameters. Several new context helper functions and option setters are added to facilitate this functionality. Changes
Possibly related PRs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
server/sse.go (4)
43-56: Return a presence flag & protect against map mutation
GetRouteParam/GetRouteParamssilently fall back to the zero value.
Returning a secondboolmakes call‑sites distinguish “missing” vs “empty string”.Additionally, the map extracted from the context is shared; a careless caller could mutate it and create data races because the same instance is reused by multiple goroutines.
-func GetRouteParam(ctx context.Context, key string) string { - if params, ok := ctx.Value(RouteParamsKey{}).(RouteParams); ok { - return params[key] - } - return "" +func GetRouteParam(ctx context.Context, key string) (string, bool) { + if params, ok := ctx.Value(routeParamsKey{}).(RouteParams); ok { + v, found := params[key] + return v, found + } + return "", false }For
GetRouteParams, consider returning a copy of the map:out := make(RouteParams, len(params)) for k, v := range params { out[k] = v } return outThat eliminates the possibility of concurrent writes on the same map.
273-274: Defensive‑copy route parameters when persisting them in the session
matchPathbuilds the params map and puts it into the request context.
handleSSEthen stores the same reference insidesseSession.routeParams, and the
later HTTP POST (handleMessage) inserts it back into a new context.If any middleware or handler mutates the map, a race occurs between the SSE goroutine and the message goroutine.
- routeParams: GetRouteParams(r.Context()), // Store route parameters from context + routeParams: func() RouteParams { + src := GetRouteParams(r.Context()) + dst := make(RouteParams, len(src)) + for k, v := range src { dst[k] = v } + return dst + }(),Do a similar clone before
context.WithValueinhandleMessage.Also applies to: 383-390
540-563:matchPathis naïve – consider pre‑compiling once‐
strings.Splitand allocations run on every request.
‐ No URL‑decoding of individual segments (e.g.,%2F).
‐ It requires exact segment count (cannot support wildcards or trailing slashes).If route patterns are static (set via
WithSSEPatternat start‑up), compile them
to a*regexp.Regexponce and store in theSSEServerstruct:re := regexp.MustCompile(`^/api/(?P<channel>[^/]+)/sse$`)Then use
re.FindStringSubmatchandre.SubexpNames()for extraction – faster
and more flexible.
471-478: Minor: helper name & error message typo
GetUrlPath→getURLPathorGetURLPathfor initialism correctness.Also, the error message currently says
failed to parse URL %s; consider quoting
with%qfor readability.examples/custom_sse_pattern/main.go (2)
22-33: Return an error when the required route param is absentThe handler treats a missing
"channel"as success (error == nil) and sends a
text result saying failed. Returning an actual error (or JSON‑RPC error) lets
clients distinguish genuine failures.- if channel == "" { - return mcp.NewToolResultText("Failed to get channel parameter"), nil - } + if channel == "" { + return nil, fmt.Errorf("missing route parameter: channel") + }This will propagate through
HandleMessageas a proper JSON‑RPC error.
49-56: Hard‑coded port may clash in CI / local testingBinding to
:8080can fail if the port is occupied.
Consider reading the port from an env‑var or CLI flag (e.g.,PORT=8080 go run …).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/custom_sse_pattern/main.go(1 hunks)server/sse.go(15 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
server/sse.go (3)
mcp/types.go (1)
Request(89-102)server/server.go (1)
MCPServer(143-166)examples/custom_context/main.go (1)
MCPServer(100-102)
🔇 Additional comments (1)
server/sse.go (1)
514-528: 404 behaviour may surprise when bothWithSSEEndpointandWithSSEPatternare setThe current logic:
- If a pattern is configured, try to match it.
- If it does not match, immediately return 404, even if the plain
/sseEndpoint
would have matched.This effectively disables the plain endpoint whenever a pattern is supplied.
Verify that this is the intended precedence; otherwise, move the “default SSE
endpoint” check in front of the 404 or fall through to it.
| // RouteParamsKey is the key type for storing route parameters in context | ||
| type RouteParamsKey struct{} | ||
|
|
||
| // RouteParams stores path parameters | ||
| type RouteParams map[string]string | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid exporting RouteParamsKey and RouteParams unless they are part of the public API
Both symbols are exported, yet they are only consumed inside server package (and the example).
Un‑exporting prevents accidental key collisions across packages and keeps your API surface minimal.
-type RouteParamsKey struct{}
-type RouteParams map[string]string
+type routeParamsKey struct{}
+type RouteParams map[string]string // keep this exported only if other pkgs really need itIf RouteParams itself is also private to server, rename it similarly (type routeParams map[string]string).
Update the occurrences accordingly.
Summary by CodeRabbit
New Features
Documentation