Severity: Warning
File: src/Servy.UI/Commands/AsyncCommand.cs, lines 30-51
AsyncCommand is the project's general-purpose IAsyncCommand implementation, used as the binding target for any async command in WPF. The ICommand.Execute entry point is async void and unprotected:
public async void Execute(object? parameter)
{
await ExecuteAsync(parameter);
}
public async Task ExecuteAsync(object? parameter)
{
if (!CanExecute(parameter)) return;
try
{
_isExecuting = true;
RaiseCanExecuteChanged();
await _execute(parameter);
}
finally
{
_isExecuting = false;
RaiseCanExecuteChanged();
}
}
ExecuteAsync has try/finally but no catch. If _execute(parameter) throws — which is the rule, not the exception, for service-control commands hitting SCM, the file system, or the network — the exception:
- Surfaces from
await _execute(parameter),
- Skips through
finally (which only resets the flag),
- Re-throws from
ExecuteAsync,
- Becomes an unobserved exception on an
async void in Execute(...),
- Posts to the synchronization context →
Dispatcher.UnhandledException in WPF.
WPF treats Dispatcher.UnhandledException as fatal unless e.Handled = true is set in a global handler. So a user clicking "Start service" while SCM is briefly unreachable risks crashing the Manager / Servy UI instead of seeing a friendly error.
Suggested fix:
The idiomatic AsyncCommand pattern is to surface the exception through Execute (async void) into a controlled handler, while leaving ExecuteAsync re-throwing for callers who explicitly await it:
public async void Execute(object? parameter)
{
try
{
await ExecuteAsync(parameter);
}
catch (Exception ex)
{
// Log and surface — never let an async void exception escape into the dispatcher.
Logger.Error(\"AsyncCommand execution failed.\", ex);
// Optionally re-raise via a configurable error handler so the consuming
// ViewModel can show a MessageBox without the framework crashing.
_onError?.Invoke(ex);
}
}
A second option: keep ExecuteAsync as-is and add catch (Exception ex) { Logger.Error(...); } directly inside ExecuteAsync before the existing finally. Either way, the async void boundary should never let exceptions reach the dispatcher unhandled.
Bonus check: a quick grep across the repo (async void) shows OnProcessExited, CheckHealth, and the two OnTick handlers all wrap their bodies in try/catch — only AsyncCommand.Execute is unprotected.
Severity: Warning
File:
src/Servy.UI/Commands/AsyncCommand.cs, lines 30-51AsyncCommandis the project's general-purposeIAsyncCommandimplementation, used as the binding target for any async command in WPF. TheICommand.Executeentry point isasync voidand unprotected:ExecuteAsynchas try/finally but nocatch. If_execute(parameter)throws — which is the rule, not the exception, for service-control commands hitting SCM, the file system, or the network — the exception:await _execute(parameter),finally(which only resets the flag),ExecuteAsync,async voidinExecute(...),Dispatcher.UnhandledExceptionin WPF.WPF treats
Dispatcher.UnhandledExceptionas fatal unlesse.Handled = trueis set in a global handler. So a user clicking "Start service" while SCM is briefly unreachable risks crashing the Manager / Servy UI instead of seeing a friendly error.Suggested fix:
The idiomatic AsyncCommand pattern is to surface the exception through
Execute(async void) into a controlled handler, while leavingExecuteAsyncre-throwing for callers who explicitly await it:A second option: keep
ExecuteAsyncas-is and addcatch (Exception ex) { Logger.Error(...); }directly insideExecuteAsyncbefore the existingfinally. Either way, theasync voidboundary should never let exceptions reach the dispatcher unhandled.Bonus check: a quick grep across the repo (
async void) showsOnProcessExited,CheckHealth, and the twoOnTickhandlers all wrap their bodies in try/catch — onlyAsyncCommand.Executeis unprotected.