-
-
Notifications
You must be signed in to change notification settings - Fork 353
feat: SessionExpired auto retry (#232) #337
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -838,6 +838,34 @@ private async Task<FunctionResultContent> InvokeFunctionAsync( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resultContent = await content.InvokeAsync(context.Kernel, cancellationToken); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| catch (Exception ex) when (context.ChatFunction is McpChatFunction && IsMcpSessionExpiredException(ex)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var mcpPlugin = _chatPluginManager.McpPlugins | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .AsValueEnumerable() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .FirstOrDefault(p => p.Name == context.ChatPlugin.Name) ?? throw new InvalidOperationException($"MCP plugin '{context.ChatPlugin.Name}' not found for session recovery."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await _chatPluginManager.StopMcpClientAsync(mcpPlugin); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await _chatPluginManager.StartMcpClientAsync(mcpPlugin, cancellationToken); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var newFunction = mcpPlugin.GetEnabledFunctions() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .OfType<McpChatFunction>() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .FirstOrDefault(f => f.KernelFunction.Name == content.FunctionName) ?? throw new InvalidOperationException($"Function '{content.FunctionName}' not found after MCP client restart."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var kernelArgs = content.Arguments != null ? new KernelArguments(content.Arguments) : []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var retryResult = await newFunction.KernelFunction.InvokeAsync(context.Kernel, kernelArgs, cancellationToken); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resultContent = new FunctionResultContent(content, retryResult); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+853
to
+859
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var newFunction = mcpPlugin.GetEnabledFunctions() | |
| .OfType<McpChatFunction>() | |
| .FirstOrDefault(f => f.KernelFunction.Name == content.FunctionName) ?? throw new InvalidOperationException($"Function '{content.FunctionName}' not found after MCP client restart."); | |
| var kernelArgs = content.Arguments != null ? new KernelArguments(content.Arguments) : []; | |
| var retryResult = await newFunction.KernelFunction.InvokeAsync(context.Kernel, kernelArgs, cancellationToken); | |
| resultContent = new FunctionResultContent(content, retryResult); | |
| _ = mcpPlugin.GetEnabledFunctions() | |
| .OfType<McpChatFunction>() | |
| .FirstOrDefault(f => f.KernelFunction.Name == content.FunctionName) ?? throw new InvalidOperationException($"Function '{content.FunctionName}' not found after MCP client restart."); | |
| resultContent = await content.InvokeAsync(context.Kernel, cancellationToken); |
Copilot
AI
Apr 10, 2026
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.
IsMcpSessionExpiredException relies on substring matching in exception messages ("Session" + "Expired"). This is brittle (message text can change across library versions, transports, or localization) and can lead to both false negatives and false positives. Prefer detecting session-expired via structured signals where available (e.g., a specific MCP/JSON-RPC error code/type, or an HTTP status like 401/403 if surfaced as an HttpRequestException), and only fall back to message matching if there is no better discriminator.
| var message = current.Message; | |
| if (message.Contains("Session", StringComparison.OrdinalIgnoreCase) && | |
| message.Contains("Expired", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| return true; | |
| } | |
| } | |
| return false; | |
| } | |
| if (TryGetAuthFailureStatusCode(current, out var statusCode) && | |
| (statusCode == System.Net.HttpStatusCode.Unauthorized || | |
| statusCode == System.Net.HttpStatusCode.Forbidden)) | |
| { | |
| return true; | |
| } | |
| if (HasSessionExpiredErrorCode(current)) | |
| { | |
| return true; | |
| } | |
| if (LooksLikeSessionExpiredMessage(current.Message)) | |
| { | |
| return true; | |
| } | |
| } | |
| return false; | |
| } | |
| private static bool TryGetAuthFailureStatusCode( | |
| Exception ex, | |
| out System.Net.HttpStatusCode statusCode) | |
| { | |
| if (ex is System.Net.Http.HttpRequestException { StatusCode: { } httpStatusCode }) | |
| { | |
| statusCode = httpStatusCode; | |
| return true; | |
| } | |
| var statusCodeValue = GetExceptionPropertyValue(ex, "StatusCode") ?? | |
| GetExceptionPropertyValue(ex, "ResponseStatusCode"); | |
| switch (statusCodeValue) | |
| { | |
| case int intCode when Enum.IsDefined(typeof(System.Net.HttpStatusCode), intCode): | |
| statusCode = (System.Net.HttpStatusCode)intCode; | |
| return true; | |
| case long longCode when longCode >= int.MinValue && | |
| longCode <= int.MaxValue && | |
| Enum.IsDefined(typeof(System.Net.HttpStatusCode), (int)longCode): | |
| statusCode = (System.Net.HttpStatusCode)(int)longCode; | |
| return true; | |
| case System.Net.HttpStatusCode enumCode: | |
| statusCode = enumCode; | |
| return true; | |
| case string textCode when int.TryParse(textCode, out var parsedCode) && | |
| Enum.IsDefined(typeof(System.Net.HttpStatusCode), parsedCode): | |
| statusCode = (System.Net.HttpStatusCode)parsedCode; | |
| return true; | |
| case string textCode when Enum.TryParse<System.Net.HttpStatusCode>(textCode, true, out var parsedStatusCode): | |
| statusCode = parsedStatusCode; | |
| return true; | |
| default: | |
| statusCode = default; | |
| return false; | |
| } | |
| } | |
| private static bool HasSessionExpiredErrorCode(Exception ex) | |
| { | |
| return IsSessionExpiredErrorCodeValue(GetExceptionPropertyValue(ex, "ErrorCode")) || | |
| IsSessionExpiredErrorCodeValue(GetExceptionPropertyValue(ex, "Code")); | |
| } | |
| private static object? GetExceptionPropertyValue(Exception ex, string propertyName) | |
| { | |
| return ex.GetType().GetProperty(propertyName)?.GetValue(ex); | |
| } | |
| private static bool IsSessionExpiredErrorCodeValue(object? value) | |
| { | |
| switch (value) | |
| { | |
| case null: | |
| return false; | |
| case int intCode: | |
| return intCode is 401 or 403; | |
| case long longCode: | |
| return longCode is 401L or 403L; | |
| case string stringCode: | |
| return string.Equals(stringCode, "session_expired", StringComparison.OrdinalIgnoreCase) || | |
| string.Equals(stringCode, "sessionexpired", StringComparison.OrdinalIgnoreCase) || | |
| string.Equals(stringCode, "unauthorized", StringComparison.OrdinalIgnoreCase) || | |
| string.Equals(stringCode, "forbidden", StringComparison.OrdinalIgnoreCase); | |
| default: | |
| var enumName = value.ToString(); | |
| return string.Equals(enumName, "Unauthorized", StringComparison.OrdinalIgnoreCase) || | |
| string.Equals(enumName, "Forbidden", StringComparison.OrdinalIgnoreCase) || | |
| string.Equals(enumName, "SessionExpired", StringComparison.OrdinalIgnoreCase); | |
| } | |
| } | |
| private static bool LooksLikeSessionExpiredMessage(string? message) | |
| { | |
| return !string.IsNullOrWhiteSpace(message) && | |
| Regex.IsMatch( | |
| message, | |
| @"\bsession\s+expired\b", | |
| RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); | |
| } |
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.
After restarting the MCP client, the code looks up the function by
content.FunctionNamein the reloadedmcpPlugin.GetEnabledFunctions(). However, function names can be deduplicated/renamed in the plugin scope by mutatingKernelFunction.Metadata.Name(see ChatPluginManager.ChatPluginSnapshot.EnsureUniqueFunctionName, where names get a_1suffix). A restart recreates MCP tools with their original names, so the retry lookup can fail even though the tool exists, causing recovery to fail with "Function ... not found after MCP client restart.". Consider preserving a stable MCP-tool identifier/name for retries (separate from the deduped display/kernel name) or reapplying the same deduplication and refreshing the kernel/plugin scope before retrying so the names match.