Skip to content

[Correctness] Manager ServiceCommands.cs — SearchServicesAsync does not filter nulls returned by ToModelAsync #797

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Warning

File: src/Servy.Manager/Services/ServiceCommands.cs lines 129-144
Related: src/Servy.Manager/Mappers/ServiceMapper.cs line 33

Description:

ServiceMapper.ToModelAsync returns null when its service argument is null:

// src/Servy.Manager/Mappers/ServiceMapper.cs:31-33
public static async Task<Service> ToModelAsync(Core.Domain.Service service, bool isDesktopAppAvailable, bool calculatePerf)
{
    if (service == null) return null;
    ...
}

SearchServicesAsync then calls it via Task.WhenAll and returns the result list verbatim — never filtering nulls:

// src/Servy.Manager/Services/ServiceCommands.cs:137-143
var tasks = results.Select(r => ServiceMapper.ToModelAsync(
    Core.Mappers.ServiceMapper.ToDomain(_serviceManager, r),
    app.IsDesktopAppAvailable,
    calculatePerf));
var services = await Task.WhenAll(tasks);

return services.ToList();

If any element in results maps to a null domain Service (e.g. Core.Mappers.ServiceMapper.ToDomain returns null for a malformed or orphaned DTO row — a case seen before in the closed #613), a null Service ends up in the list. The return type is List<Service>, so downstream UI binding code that does not explicitly null-check (e.g. .Name access in XAML templates) will NPE deep inside the render pass — far from the cause.

Suggested fix:

Filter nulls after Task.WhenAll, or change ToModelAsync to throw ArgumentNullException on null input (fail fast — aligns better with the non-nullable return type):

var services = (await Task.WhenAll(tasks)).Where(s => s != null).ToList();
return services;

If the null-domain path is truly unreachable in current code, the defensive if (service == null) return null; in ToModelAsync should be removed so callers can rely on the non-null contract.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions