-
Notifications
You must be signed in to change notification settings - Fork 21
chore: upgrade go-sdk to v1.5.0 and address go-fan review items #3610
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
05a8efe
Initial plan
Copilot 6b3df9a
chore: upgrade go-sdk to v1.5.0
Copilot 2d002ac
fix: replace string-matching session error with sdk.ErrSessionMissing…
Copilot 53b7390
Address review: add sentinel test cases, use structured logger for SS…
lpcox File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,12 +67,17 @@ func isHTTPConnectionError(err error) bool { | |
| return false | ||
| } | ||
|
|
||
| // isSessionNotFoundError checks if an error message indicates a backend MCP session has expired | ||
| // isSessionNotFoundError checks if an error indicates a backend MCP session has expired | ||
| // or is not found. This is used to detect when automatic reconnection to the backend is needed. | ||
| // It uses errors.Is to check for sdk.ErrSessionMissing (the typed sentinel introduced in v1.5.0), | ||
| // and also falls back to string-matching for non-SDK transports that return plain error messages. | ||
| func isSessionNotFoundError(err error) bool { | ||
| if err == nil { | ||
| return false | ||
| } | ||
| if errors.Is(err, sdk.ErrSessionMissing) { | ||
| return true | ||
| } | ||
|
Comment on lines
74
to
+80
|
||
| return strings.Contains(strings.ToLower(err.Error()), "session not found") | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
logConn.Printfis DEBUG-gated (and logs at debug level to the file logger), so this message will no longer be visible by default (unlike the previouslog.Printf) and may not land inmcp-gateway.logat WARN/INFO. If this line is intended as part of the SSE deprecation/operator visibility, route it through the structured logger too (e.g.,logger.LogWarn/LogInfo) for consistency with the other deprecation messages.