-
Notifications
You must be signed in to change notification settings - Fork 339
feat(SSEServer): add WithAppendQueryToMessageEndpoint() #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces a new boolean field Changes
Possibly related PRs
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/sse.go (1)
309-314
: Consider a more robust approach for appending query parameters.While the current implementation works correctly with the existing
GetMessageEndpointForClient
function (which always includes asessionId
parameter), it could be more robust to handle edge cases:
- The code assumes the endpoint already contains parameters and always uses
&
as a separator- If the request query already contains a
sessionId
parameter, this would create duplicate parametersConsider this more robust implementation:
endpoint := s.GetMessageEndpointForClient(sessionID) if s.appendQueryToMessageEndpoint && len(r.URL.RawQuery) > 0 { - endpoint += "&" + r.URL.RawQuery + // Parse the original query to potentially filter parameters + originalQuery, _ := url.ParseQuery(r.URL.RawQuery) + + // Remove sessionId if present to avoid duplication + originalQuery.Del("sessionId") + + // Only append if there are remaining parameters + if len(originalQuery) > 0 { + if strings.Contains(endpoint, "?") { + endpoint += "&" + originalQuery.Encode() + } else { + endpoint += "?" + originalQuery.Encode() + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/sse.go
(4 hunks)
🔇 Additional comments (2)
server/sse.go (2)
116-125
: Well-documented function with clear purpose.The
WithAppendQueryToMessageEndpoint
function follows the established pattern of option functions and includes comprehensive documentation that clearly explains its purpose and use cases.
69-69
: Good structural placement of the new field.The new
appendQueryToMessageEndpoint
boolean field is appropriately placed at the end of the struct definition, maintaining backward compatibility and following the established pattern for feature flags in this struct.
65b2789
to
3b27053
Compare
Hi there, I hope you're doing well. I'm reaching out to kindly request the review and merge of PR #136 as soon as possible. This change is critical for our business operations, and we're currently blocked pending its inclusion in the main branch. We really appreciate the work you're doing on this project and would be extremely grateful if this PR could be prioritized. Please let me know if there's anything I can do to help expedite the process. Thanks in advance for your time and support! Best regards |
configures the SSE server to append the original request's RawQuery to message endpoint
3b27053
to
efe46f7
Compare
Configures the SSE server to append the original request's RawQuery to message endpoint.
The
WithAppendQueryToMessageEndpoint
configures the SSE server to append the query string of the original request to the message endpoint URL sent to the client during SSE connection initialization. This is useful when it is necessary to retain the query parameters from the initial SSE connection request and pass them on to subsequent message requests, thereby maintaining context, target orientation, and authentication details throughout the communication channel.For examples:
Summary by CodeRabbit