Skip to content

fix(application): typed Result<T>.Failure on validation failures (POM-482)#102

Merged
Pomdapis merged 1 commit into
mainfrom
fix/application-validation-behavior-result-null
May 16, 2026
Merged

fix(application): typed Result<T>.Failure on validation failures (POM-482)#102
Pomdapis merged 1 commit into
mainfrom
fix/application-validation-behavior-result-null

Conversation

@Pomdapis
Copy link
Copy Markdown
Contributor

Summary

Fixes POM-482ValidationBehavior was returning null (cast to TResponse) for the Result<T> failure path, silently corrupting the CQRS pipeline for any handler whose response is Result<TValue>.

Root cause

src/Application/Compendium.Application/CQRS/Behaviors/ValidationBehavior.cs:40-44 did :

```csharp
var failureMethod = typeof(Result<>).MakeGenericType(resultType)
.GetMethod(nameof(Result.Failure), new[] { typeof(Error) });

var result = failureMethod?.Invoke(null, new object[] { error }); // ← null in production
return (TResponse)result!;
```

`Failure(Error)` is a static on the non-generic `Result` base class (src/Core/Compendium.Core/Results/Result.cs:88). `GetMethod` on the closed generic `Result` does not surface inherited statics without `BindingFlags.FlattenHierarchy`, so the lookup returned `null` and the null-conditional invocation silently produced `null`.

Fix

Resolve the static through the non-generic `Result` type and close it over the response's inner type via `MakeGenericMethod` :

```csharp
var failureMethod = typeof(Result)
.GetMethods(BindingFlags.Public | BindingFlags.Static)
.Single(m => m.Name == nameof(Result.Failure)
&& m.IsGenericMethodDefinition
&& m.GetParameters().Length == 1
&& m.GetParameters()[0].ParameterType == typeof(Error))
.MakeGenericMethod(resultType);
```

Test

The existing `HandleAsync_WhenInvalidAndResponseIsGenericResult_ShortCircuitsBeforeNext` was renamed to `_ReturnsFailureAndShortCircuits` and now asserts :

  • `actual.Should().NotBeNull()`
  • `actual.IsFailure.Should().BeTrue()`
  • `actual.Error.Code.Should().Be("Validation.Failed")`
  • `actual.Error.Type.Should().Be(ErrorType.Validation)`
  • `nextCalled.Should().BeFalse()`

The previous version accepted the buggy `null` with a comment — that comment is now removed.

Verification

  • `dotnet test tests/Unit/Compendium.Application.Tests` → 225/225 green locally.
  • No regression in any other Application test (LoggingBehavior, TransactionBehavior, IdempotencyBehavior, dispatchers, etc.).
  • No prod-code change outside the affected method body + a single `using System.Reflection;`.

Release impact

After merge, target framework v1.0.1 → republishes `Compendium.Application`.

Test plan

  • CI green (build + 90 % coverage gate).
  • CodeQL green.
  • User reviews and merges.

…s (POM-482)

The previous reflection lookup `typeof(Result<>).MakeGenericType(t).GetMethod("Failure", new[] { typeof(Error) })`
returned `null` because the static `Failure<T>(Error)` factory lives on the
non-generic `Result` base class. `GetMethod` on a closed generic does not
surface inherited statics without `BindingFlags.FlattenHierarchy`, so the
behavior was returning `null` cast to `Result<T>` for the failure path —
silently corrupting the CQRS pipeline for any handler whose response is
`Result<TValue>`.

Resolve the static method through the non-generic `Result` type and close
it over the response's inner type via `MakeGenericMethod`. The existing
short-circuit test is upgraded from accepting the buggy `null` to asserting
`IsFailure == true` with the proper `Validation.Failed` error code/type.

All 225 unit tests in `Compendium.Application.Tests` pass.
Copilot AI review requested due to automatic review settings May 16, 2026 08:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants