-
-
Notifications
You must be signed in to change notification settings - Fork 585
.pr_agent_auto_best_practices
Pattern 1: Ensure cooperative cancellation and cleanup by not passing cancellation tokens into Task.Run for long-running work, awaiting the task with WaitAsync(cancellationToken), and catching OperationCanceledException to allow finally blocks to restore state.
Example code before:
var task = Task.Run(() => DoWork(token), token);
return await task.WaitAsync(token); // may cancel before finally runs in task
Example code after:
var task = Task.Run(() => DoWork(token)); // don't pass token
try
{
return await task.WaitAsync(token);
}
catch (OperationCanceledException)
{
try { await task; } catch { }
throw;
}
Relevant past accepted suggestions:
Suggestion 1:
Ensure cleanup on cancellation
Refactor the task cancellation logic to ensure the finally block always executes, preventing the Python interpreter's state from being left corrupted upon cancellation. This involves removing the cancellation token from Task.Run and handling cancellation exceptions from WaitAsync to allow for cleanup.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [137-216]
private async Task<CodeInterpretResponse> CoreRunScript(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();
var execTask = Task.Run(() =>
{
using (Py.GIL())
{
- // Import necessary Python modules
dynamic sys = Py.Import("sys");
dynamic io = Py.Import("io");
try
{
- // Redirect standard output/error to capture it
dynamic outIO = io.StringIO();
dynamic errIO = io.StringIO();
sys.stdout = outIO;
sys.stderr = errIO;
- // Set global items
using var globals = new PyDict();
if (codeScript.Contains("__main__") == true)
{
globals.SetItem("__name__", new PyString("__main__"));
}
- // Set arguments
var list = new PyList();
if (options?.Arguments?.Any() == true)
{
list.Append(new PyString(options?.ScriptName ?? "script.py"));
foreach (var arg in options!.Arguments)
{
if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value))
{
list.Append(new PyString($"--{arg.Key}"));
list.Append(new PyString($"{arg.Value}"));
}
}
}
sys.argv = list;
- cancellationToken.ThrowIfCancellationRequested();
+ // cooperative cancellation point before exec
+ if (cancellationToken.IsCancellationRequested)
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ }
- // Execute Python script
PythonEngine.Exec(codeScript, globals);
- // Get result
var stdout = outIO.getvalue()?.ToString() as string;
- var stderr = errIO.getvalue()?.ToString() as string;
- cancellationToken.ThrowIfCancellationRequested();
+ // cooperative cancellation after exec
+ if (cancellationToken.IsCancellationRequested)
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ }
return new CodeInterpretResponse
{
Result = stdout?.TrimEnd('\r', '\n') ?? string.Empty,
Success = true
};
}
catch (Exception ex)
{
_logger.LogError(ex, $"Error in {nameof(CoreRunScript)} in {Provider}.");
return new() { ErrorMsg = ex.Message };
}
finally
{
- // Restore the original stdout/stderr/argv
sys.stdout = sys.__stdout__;
sys.stderr = sys.__stderr__;
sys.argv = new PyList();
}
- };
- }, cancellationToken);
+ }
+ }); // do not pass cancellationToken here
- return await execTask.WaitAsync(cancellationToken);
+ try
+ {
+ return await execTask.WaitAsync(cancellationToken);
+ }
+ catch (OperationCanceledException)
+ {
+ // Ensure the inner task completes cleanup
+ try { await execTask; } catch { /* ignored: already logged */ }
+ throw;
+ }
}Suggestion 2:
Use Task.Run for execution
Replace Task.Factory.StartNew with Task.Run for better integration with the thread pool and cancellation, and remove the redundant try-catch block and an extraneous semicolon.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [143-220]
-var execTask = Task.Factory.StartNew(() =>
+var execTask = Task.Run(() =>
{
using (Py.GIL())
{
- ...
- };
+ // existing body unchanged
+ dynamic sys = Py.Import("sys");
+ dynamic io = Py.Import("io");
+ try
+ {
+ dynamic stringIO = io.StringIO();
+ dynamic errIO = io.StringIO();
+ sys.stdout = stringIO;
+ sys.stderr = errIO;
+ using var globals = new PyDict();
+ if (codeScript.Contains("__main__") == true)
+ {
+ globals.SetItem("__name__", new PyString("__main__"));
+ }
+ var list = new PyList();
+ if (options?.Arguments?.Any() == true)
+ {
+ list.Append(new PyString(options?.ScriptName ?? "script.py"));
+ foreach (var arg in options!.Arguments)
+ {
+ if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value))
+ {
+ list.Append(new PyString($"--{arg.Key}"));
+ list.Append(new PyString($"{arg.Value}"));
+ }
+ }
+ }
+ sys.argv = list;
+
+ cancellationToken.ThrowIfCancellationRequested();
+
+ PythonEngine.Exec(codeScript, globals);
+
+ var stdout = stringIO.getvalue()?.ToString() as string;
+ var stderr = errIO.getvalue()?.ToString() as string;
+
+ cancellationToken.ThrowIfCancellationRequested();
+
+ return new CodeInterpretResponse
+ {
+ Result = (stdout ?? string.Empty).TrimEnd('\r', '\n'),
+ ErrorMsg = stderr,
+ Success = string.IsNullOrEmpty(stderr)
+ };
+ }
+ finally
+ {
+ sys.stdout = sys.__stdout__;
+ sys.stderr = sys.__stderr__;
+ sys.argv = new PyList();
+ }
+ }
}, cancellationToken);
-try
-{
- return await execTask.WaitAsync(cancellationToken);
-}
-catch
-{
- throw;
-}
+return await execTask.WaitAsync(cancellationToken);Suggestion 3:
Prevent potential process execution deadlock
Prevent a potential process deadlock by reading the standard output and error streams concurrently with waiting for the process to exit using Task.WhenAll.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [267-273]
-var stdoutTask = proc.StandardOutput.ReadToEndAsync();
-var stderrTask = proc.StandardError.ReadToEndAsync();
+var stdoutTask = proc.StandardOutput.ReadToEndAsync(token);
+var stderrTask = proc.StandardError.ReadToEndAsync(token);
-await proc.WaitForExitAsync();
+await Task.WhenAll(proc.WaitForExitAsync(token), stdoutTask, stderrTask);
var stdout = await stdoutTask;
var stderr = await stderrTask;Suggestion 4:
Check WebSocket state
The SendEventToUser method doesn't check if the WebSocket is in a valid state before sending data. If the client has disconnected or the connection is closing, this could throw an exception and crash the middleware.
src/Plugins/BotSharp.Plugin.ChatHub/ChatStreamMiddleware.cs [106]
-await SendEventToUser(webSocket, data);
+if (webSocket.State == WebSocketState.Open)
+{
+ await SendEventToUser(webSocket, data);
+}[Suggestion has been applied]
Pattern 2: Capture and return precise error details by separating stdout and stderr, surfacing stderr or exception messages in response objects, and determining Success based on the absence of stderr.
Example code before:
sys.stdout = io.StringIO()
sys.stderr = sys.stdout
// ...
return new Response { Result = stdout, Success = true };
Example code after:
var outIO = io.StringIO(); var errIO = io.StringIO();
sys.stdout = outIO; sys.stderr = errIO;
// ...
return new Response
{
Result = (outIO.getvalue() ?? "").TrimEnd('\r','\n'),
ErrorMsg = errIO.getvalue(),
Success = string.IsNullOrEmpty(errIO.getvalue())
};
Relevant past accepted suggestions:
Suggestion 1:
Preserve and return error details
Improve error handling in CoreRunScript by capturing and returning specific error details from stderr or exceptions in the CodeInterpretResponse, instead of returning a generic empty response on failure.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [137-221]
private async Task<CodeInterpretResponse> CoreRunScript(string codeScript, CodeInterpretOptions? options = null, CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();
var execTask = Task.Factory.StartNew(() =>
{
using (Py.GIL())
{
- // Import necessary Python modules
dynamic sys = Py.Import("sys");
dynamic io = Py.Import("io");
try
{
- // Redirect standard output/error to capture it
dynamic stringIO = io.StringIO();
+ dynamic errIO = io.StringIO();
sys.stdout = stringIO;
- sys.stderr = stringIO;
+ sys.stderr = errIO;
- // Set global items
using var globals = new PyDict();
if (codeScript.Contains("__main__") == true)
{
globals.SetItem("__name__", new PyString("__main__"));
}
- // Set arguments
var list = new PyList();
if (options?.Arguments?.Any() == true)
{
list.Append(new PyString(options?.ScriptName ?? "script.py"));
-
foreach (var arg in options!.Arguments)
{
if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value))
{
list.Append(new PyString($"--{arg.Key}"));
list.Append(new PyString($"{arg.Value}"));
}
}
}
sys.argv = list;
cancellationToken.ThrowIfCancellationRequested();
- // Execute Python script
PythonEngine.Exec(codeScript, globals);
- // Get result
- var result = stringIO.getvalue()?.ToString() as string;
+ var stdout = stringIO.getvalue()?.ToString() as string;
+ var stderr = errIO.getvalue()?.ToString() as string;
cancellationToken.ThrowIfCancellationRequested();
return new CodeInterpretResponse
{
- Result = result?.TrimEnd('\r', '\n') ?? string.Empty,
- Success = true
+ Result = (stdout ?? string.Empty).TrimEnd('\r', '\n'),
+ ErrorMsg = stderr,
+ Success = string.IsNullOrEmpty(stderr)
};
}
catch (Exception ex)
{
_logger.LogError(ex, $"Error in {nameof(CoreRunScript)} in {Provider}.");
- return new();
+ return new CodeInterpretResponse
+ {
+ Success = false,
+ ErrorMsg = ex.Message
+ };
}
finally
{
- // Restore the original stdout/stderr/argv
sys.stdout = sys.__stdout__;
sys.stderr = sys.__stderr__;
sys.argv = new PyList();
}
- };
+ }
}, cancellationToken);
- try
- {
- return await execTask.WaitAsync(cancellationToken);
- }
- catch
- {
- throw;
- }
+ return await execTask.WaitAsync(cancellationToken);
}Suggestion 2:
Use Task.Run for execution
Replace Task.Factory.StartNew with Task.Run for better integration with the thread pool and cancellation, and remove the redundant try-catch block and an extraneous semicolon.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [143-220]
-var execTask = Task.Factory.StartNew(() =>
+var execTask = Task.Run(() =>
{
using (Py.GIL())
{
- ...
- };
+ // existing body unchanged
+ dynamic sys = Py.Import("sys");
+ dynamic io = Py.Import("io");
+ try
+ {
+ dynamic stringIO = io.StringIO();
+ dynamic errIO = io.StringIO();
+ sys.stdout = stringIO;
+ sys.stderr = errIO;
+ using var globals = new PyDict();
+ if (codeScript.Contains("__main__") == true)
+ {
+ globals.SetItem("__name__", new PyString("__main__"));
+ }
+ var list = new PyList();
+ if (options?.Arguments?.Any() == true)
+ {
+ list.Append(new PyString(options?.ScriptName ?? "script.py"));
+ foreach (var arg in options!.Arguments)
+ {
+ if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value))
+ {
+ list.Append(new PyString($"--{arg.Key}"));
+ list.Append(new PyString($"{arg.Value}"));
+ }
+ }
+ }
+ sys.argv = list;
+
+ cancellationToken.ThrowIfCancellationRequested();
+
+ PythonEngine.Exec(codeScript, globals);
+
+ var stdout = stringIO.getvalue()?.ToString() as string;
+ var stderr = errIO.getvalue()?.ToString() as string;
+
+ cancellationToken.ThrowIfCancellationRequested();
+
+ return new CodeInterpretResponse
+ {
+ Result = (stdout ?? string.Empty).TrimEnd('\r', '\n'),
+ ErrorMsg = stderr,
+ Success = string.IsNullOrEmpty(stderr)
+ };
+ }
+ finally
+ {
+ sys.stdout = sys.__stdout__;
+ sys.stderr = sys.__stderr__;
+ sys.argv = new PyList();
+ }
+ }
}, cancellationToken);
-try
-{
- return await execTask.WaitAsync(cancellationToken);
-}
-catch
-{
- throw;
-}
+return await execTask.WaitAsync(cancellationToken);Suggestion 3:
Prevent potential process execution deadlock
Prevent a potential process deadlock by reading the standard output and error streams concurrently with waiting for the process to exit using Task.WhenAll.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [267-273]
-var stdoutTask = proc.StandardOutput.ReadToEndAsync();
-var stderrTask = proc.StandardError.ReadToEndAsync();
+var stdoutTask = proc.StandardOutput.ReadToEndAsync(token);
+var stderrTask = proc.StandardError.ReadToEndAsync(token);
-await proc.WaitForExitAsync();
+await Task.WhenAll(proc.WaitForExitAsync(token), stdoutTask, stderrTask);
var stdout = await stdoutTask;
var stderr = await stderrTask;Suggestion 4:
Prevent null reference exception
The code uses a null conditional operator on agent but doesn't check if agent is null before accessing its Description property. If agent is null, this could lead to a NullReferenceException when trying to access agent.Description.
src/Plugins/BotSharp.Plugin.OpenAI/Providers/Realtime/RealTimeCompletionProvider.cs [283]
-var instruction = messages.FirstOrDefault()?.Content.FirstOrDefault()?.Text ?? agent?.Description;
+var instruction = messages.FirstOrDefault()?.Content.FirstOrDefault()?.Text ?? agent?.Description ?? string.Empty;Pattern 3: Guard against nulls, empty collections, and invalid states before access by using explicit checks (nullables, Any(), EndOfMessage, WebSocketState.Open) to avoid NREs and out-of-range errors.
Example code before:
var item = list.ElementAt(random.Next(0, list.Count()));
var text = Encoding.UTF8.GetString(buffer, 0, result.Count); // per-frame
await socket.SendAsync(data); // without state check
Example code after:
var items = list.ToList();
if (!items.Any()) return null;
var idx = random.Next(0, items.Count);
// Accumulate frames until EndOfMessage, then decode
if (socket.State == WebSocketState.Open) await socket.SendAsync(data);
Relevant past accepted suggestions:
Suggestion 1:
Prevent crash when no models match
Prevent a potential ArgumentOutOfRangeException in GetProviderModel by checking if the filtered models collection is empty. If it is, return null instead of attempting to select an element, which would cause a crash.
src/Infrastructure/BotSharp.Core/Infrastructures/LlmProviderService.cs [61-69]
if (capabilities != null)
{
models = models.Where(x => x.Capabilities != null && capabilities.Any(y => x.Capabilities.Contains(y)));
}
+var availableModels = models.ToList();
+if (!availableModels.Any())
+{
+ return null;
+}
+
var random = new Random();
-var index = random.Next(0, models.Count());
-var modelSetting = models.ElementAt(index);
+var index = random.Next(0, availableModels.Count);
+var modelSetting = availableModels.ElementAt(index);
return modelSetting;Suggestion 2:
Use case-insensitive model ID filtering
Update the model ID filtering logic in GetLlmConfigs to be case-insensitive. Use StringComparer.OrdinalIgnoreCase when checking if filter.ModelIds contains a model's ID to ensure consistent and robust filtering.
src/Infrastructure/BotSharp.Core/Infrastructures/LlmProviderService.cs [139-142]
if (filter.ModelIds != null)
{
- models = models.Where(x => filter.ModelIds.Contains(x.Id));
+ models = models.Where(x => filter.ModelIds.Contains(x.Id, StringComparer.OrdinalIgnoreCase));
}Suggestion 3:
Fix comparer misuse and null checks
Avoid passing a comparer to HashSet.Contains — it only accepts the element. Also, null-check function names to prevent NREs. Build the set with the comparer and call Contains with a single argument, and ensure x.Name is not null before accessing.
src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.Rendering.cs [110-124]
public IEnumerable<FunctionDef> FilterFunctions(string instruction, Agent agent, StringComparer? comparer = null)
{
var functions = agent.Functions.AsEnumerable();
if (agent.FuncVisMode.IsEqualTo(AgentFuncVisMode.Auto) && !string.IsNullOrWhiteSpace(instruction))
{
- comparer = comparer ?? StringComparer.OrdinalIgnoreCase;
+ comparer ??= StringComparer.OrdinalIgnoreCase;
var matches = Regex.Matches(instruction, @"\b[A-Za-z0-9_]+\b");
var words = new HashSet<string>(matches.Select(m => m.Value), comparer);
- functions = functions.Where(x => words.Contains(x.Name, comparer));
+ functions = functions.Where(x => !string.IsNullOrEmpty(x.Name) && words.Contains(x.Name));
}
functions = functions.Concat(agent.SecondaryFunctions ?? []);
return functions;
}Suggestion 4:
Fix WebSocket fragmentation handling
The code is using a fixed-size buffer for receiving WebSocket messages, but it doesn't handle fragmented messages correctly. WebSocket messages can be split across multiple frames, and the current implementation will process each frame independently, potentially causing data corruption for large messages.
src/Plugins/BotSharp.Plugin.ChatHub/ChatStreamMiddleware.cs [61-77]
var buffer = new byte[1024 * 32];
WebSocketReceiveResult result;
+var messageBuffer = new List<byte>();
do
{
result = await webSocket.ReceiveAsync(new(buffer), CancellationToken.None);
if (result.MessageType != WebSocketMessageType.Text)
{
continue;
}
- var receivedText = Encoding.UTF8.GetString(buffer, 0, result.Count);
- if (string.IsNullOrEmpty(receivedText))
+ messageBuffer.AddRange(new ArraySegment<byte>(buffer, 0, result.Count));
+
+ if (result.EndOfMessage)
{
- continue;
- }
+ var receivedText = Encoding.UTF8.GetString(messageBuffer.ToArray());
+ messageBuffer.Clear();
+
+ if (string.IsNullOrEmpty(receivedText))
+ {
+ continue;
+ }Suggestion 5:
Check WebSocket state
The SendEventToUser method doesn't check if the WebSocket is in a valid state before sending data. If the client has disconnected or the connection is closing, this could throw an exception and crash the middleware.
src/Plugins/BotSharp.Plugin.ChatHub/ChatStreamMiddleware.cs [106]
-await SendEventToUser(webSocket, data);
+if (webSocket.State == WebSocketState.Open)
+{
+ await SendEventToUser(webSocket, data);
+}[Suggestion has been applied]
Suggestion 6:
Prevent null reference exception
The code uses a null conditional operator on agent but doesn't check if agent is null before accessing its Description property. If agent is null, this could lead to a NullReferenceException when trying to access agent.Description.
src/Plugins/BotSharp.Plugin.OpenAI/Providers/Realtime/RealTimeCompletionProvider.cs [283]
-var instruction = messages.FirstOrDefault()?.Content.FirstOrDefault()?.Text ?? agent?.Description;
+var instruction = messages.FirstOrDefault()?.Content.FirstOrDefault()?.Text ?? agent?.Description ?? string.Empty;Pattern 4: Use appropriate APIs and logic for async and stream/process operations: prefer Task.Run over Task.Factory.StartNew for CPU-bound work, read stdout/stderr concurrently with Task.WhenAll, avoid sync waits on tasks, and remove unnecessary async when no await is present.
Example code before:
var execTask = Task.Factory.StartNew(() => DoExec(), token);
var stdout = await proc.StandardOutput.ReadToEndAsync();
await proc.WaitForExitAsync();
var res = someAsync().Result; // sync wait
Example code after:
var execTask = Task.Run(() => DoExec());
var stdoutTask = proc.StandardOutput.ReadToEndAsync(token);
var stderrTask = proc.StandardError.ReadToEndAsync(token);
await Task.WhenAll(proc.WaitForExitAsync(token), stdoutTask, stderrTask);
var res = someAsync().GetAwaiter().GetResult(); // or fully async, and remove async if no await
Relevant past accepted suggestions:
Suggestion 1:
Use Task.Run for execution
Replace Task.Factory.StartNew with Task.Run for better integration with the thread pool and cancellation, and remove the redundant try-catch block and an extraneous semicolon.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [143-220]
-var execTask = Task.Factory.StartNew(() =>
+var execTask = Task.Run(() =>
{
using (Py.GIL())
{
- ...
- };
+ // existing body unchanged
+ dynamic sys = Py.Import("sys");
+ dynamic io = Py.Import("io");
+ try
+ {
+ dynamic stringIO = io.StringIO();
+ dynamic errIO = io.StringIO();
+ sys.stdout = stringIO;
+ sys.stderr = errIO;
+ using var globals = new PyDict();
+ if (codeScript.Contains("__main__") == true)
+ {
+ globals.SetItem("__name__", new PyString("__main__"));
+ }
+ var list = new PyList();
+ if (options?.Arguments?.Any() == true)
+ {
+ list.Append(new PyString(options?.ScriptName ?? "script.py"));
+ foreach (var arg in options!.Arguments)
+ {
+ if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value))
+ {
+ list.Append(new PyString($"--{arg.Key}"));
+ list.Append(new PyString($"{arg.Value}"));
+ }
+ }
+ }
+ sys.argv = list;
+
+ cancellationToken.ThrowIfCancellationRequested();
+
+ PythonEngine.Exec(codeScript, globals);
+
+ var stdout = stringIO.getvalue()?.ToString() as string;
+ var stderr = errIO.getvalue()?.ToString() as string;
+
+ cancellationToken.ThrowIfCancellationRequested();
+
+ return new CodeInterpretResponse
+ {
+ Result = (stdout ?? string.Empty).TrimEnd('\r', '\n'),
+ ErrorMsg = stderr,
+ Success = string.IsNullOrEmpty(stderr)
+ };
+ }
+ finally
+ {
+ sys.stdout = sys.__stdout__;
+ sys.stderr = sys.__stderr__;
+ sys.argv = new PyList();
+ }
+ }
}, cancellationToken);
-try
-{
- return await execTask.WaitAsync(cancellationToken);
-}
-catch
-{
- throw;
-}
+return await execTask.WaitAsync(cancellationToken);Suggestion 2:
Prevent potential process execution deadlock
Prevent a potential process deadlock by reading the standard output and error streams concurrently with waiting for the process to exit using Task.WhenAll.
src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [267-273]
-var stdoutTask = proc.StandardOutput.ReadToEndAsync();
-var stderrTask = proc.StandardError.ReadToEndAsync();
+var stdoutTask = proc.StandardOutput.ReadToEndAsync(token);
+var stderrTask = proc.StandardError.ReadToEndAsync(token);
-await proc.WaitForExitAsync();
+await Task.WhenAll(proc.WaitForExitAsync(token), stdoutTask, stderrTask);
var stdout = await stdoutTask;
var stderr = await stderrTask;Suggestion 3:
Remove unnecessary async and add guard
The method is declared async but contains no await, which can introduce unnecessary state machine overhead and warnings. Also, accessing page.Url may throw if the page is disposed; wrap in a try-catch and return an empty string on failure to align with the null-path behavior.
-public async Task<string> GetCurrentUrl(MessageInfo message)
+public Task<string> GetCurrentUrl(MessageInfo message)
{
- var page = _instance.GetPage(message.ContextId);
- if (page == null)
- return string.Empty;
- return page.Url;
+ try
+ {
+ var page = _instance.GetPage(message.ContextId);
+ if (page == null) return Task.FromResult(string.Empty);
+ return Task.FromResult(page.Url ?? string.Empty);
+ }
+ catch
+ {
+ return Task.FromResult(string.Empty);
+ }
}Suggestion 4:
Avoid potential deadlocks
The code is using .Result to synchronously wait for an asynchronous operation, which can lead to deadlocks in ASP.NET applications. This is a common anti-pattern that should be avoided.
src/Infrastructure/BotSharp.Core/Routing/RoutingContext.cs [85-93]
// Convert id to name
if (!Guid.TryParse(agentId, out _))
{
var agentService = _services.GetRequiredService<IAgentService>();
- var agents = agentService.GetAgentOptions([agentId]).Result;
+ var agents = agentService.GetAgentOptions([agentId]).GetAwaiter().GetResult();
if (agents.Count > 0)
{
agentId = agents.First().Id;[Suggestion has been applied]
[Auto-generated best practices - 2025-11-12]