GH-4516: map an unknown tenant id to a 404 ProblemDetails instead of a 500

docs/guide/http/exception-handling.md

@@ -248,6 +248,39 @@ methods are optimistic and throw `EventStreamUnexpectedMaxEventIdException`, whi
 `ConcurrencyException`.
 :::
 
+## Unknown Tenants as 404 <Badge type="tip" text="6.39" />
+
+There are two different tenancy failures on an HTTP request, and only one of them was handled:
+
+| Failure | Meaning | Status |
+| --- | --- | --- |
+| **Missing** tenant id | "you did not say which tenant" | 400, already handled by `[RequiresTenant]` / `TenantId.AssertExists()` |
+| **Unknown** tenant id | "the tenant you named does not exist" | was an unhandled **500** |
+
+An unknown tenant id throws `JasperFx.MultiTenancy.UnknownTenantIdException` from the store or from
+Wolverine's own tenant sources. That is a client side error, so:
+
+```csharp
+app.MapWolverineEndpoints(opts =>
+{
+    opts.MapUnknownTenantToNotFound();
+});
+```
+
+maps it to a 404 `ProblemDetails` titled `Unknown tenant`, and stamps `ProducesProblem(404)` on the tenanted
+chains so your OpenAPI document advertises it. It applies by default to chains declared `[RequiresTenant]` or
+`[MaybeTenanted]` -- a chain that resolves no tenant cannot fail to resolve one -- and takes the same optional
+predicate the other mappings do.
+
+::: tip Why 404 and not 400
+404 reads as "the thing you addressed does not exist", which keeps 400 meaning "you did not say which
+tenant". Collapsing both onto one status loses the distinction a caller needs to tell a routing bug from a
+provisioning one.
+:::
+
+For **message handlers** the equivalent guidance is `OnException<UnknownTenantIdException>().MoveToErrorQueue()`
+-- never retry it, since it is deterministic.
+
 ### Rolling your own
 
 If you want different status codes, a different `ProblemDetails` shape, or extra exception types, the

src/Http/Wolverine.Http.Tests/unknown_tenant_problem_details_4516.cs

@@ -0,0 +1,155 @@
+using Alba;
+using JasperFx.MultiTenancy;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Http.Metadata;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Shouldly;
+using Xunit;
+
+namespace Wolverine.Http.Tests;
+
+/// <summary>
+/// GH-4516. A <b>missing</b> mandatory tenant id was already handled -- [RequiresTenant] stops the request
+/// with a 400 ProblemDetails. An <b>unknown</b> one, present on the request but with no database or
+/// registration behind it, threw UnknownTenantIdException, which Wolverine.Http did not catch, so the client
+/// got a 500 for what is a client side error.
+/// </summary>
+public class unknown_tenant_problem_details_4516
+{
+    [Fact]
+    public async Task an_unknown_tenant_id_is_a_404_problem_details()
+    {
+        await using var host = await startHostAsync(mapUnknownTenant: true);
+
+        var result = await host.Scenario(x =>
+        {
+            x.Get.Url("/gh4516/tenanted?tenantId=ghost");
+            x.StatusCodeShouldBe(404);
+        });
+
+        var problem = await result.ReadAsJsonAsync<ProblemDetails>();
+
+        problem.ShouldNotBeNull();
+        problem.Status.ShouldBe(404);
+        problem.Title.ShouldBe("Unknown tenant");
+        problem.Detail.ShouldNotBeNull();
+        problem.Detail.ShouldContain("ghost");
+    }
+
+    [Fact]
+    public async Task a_known_tenant_is_untouched()
+    {
+        await using var host = await startHostAsync(mapUnknownTenant: true);
+
+        var result = await host.Scenario(x =>
+        {
+            x.Get.Url("/gh4516/tenanted?tenantId=acme");
+            x.StatusCodeShouldBeOk();
+        });
+
+        (await result.ReadAsTextAsync()).ShouldBe("acme");
+    }
+
+    [Fact]
+    public async Task a_missing_tenant_id_still_answers_400_not_404()
+    {
+        // The two failures are genuinely different -- "you did not say which tenant" versus "the tenant you
+        // named does not exist" -- and this mapping must not collapse them onto one status.
+        await using var host = await startHostAsync(mapUnknownTenant: true);
+
+        await host.Scenario(x =>
+        {
+            x.Get.Url("/gh4516/tenanted");
+            x.StatusCodeShouldBe(400);
+        });
+    }
+
+    [Fact]
+    public async Task without_the_opt_in_the_unknown_tenant_still_escapes()
+    {
+        // Proves the opt in is what changes the answer, rather than something else in the pipeline.
+        await using var host = await startHostAsync(mapUnknownTenant: false);
+
+        await Should.ThrowAsync<UnknownTenantIdException>(async () =>
+            await host.Scenario(x =>
+            {
+                x.Get.Url("/gh4516/tenanted?tenantId=ghost");
+                x.StatusCodeShouldBe(404);
+            }));
+    }
+
+    [Fact]
+    public async Task the_404_is_advertised_only_on_the_tenanted_chains()
+    {
+        await using var host = await startHostAsync(mapUnknownTenant: true);
+
+        var chains = host.Services.GetRequiredService<WolverineHttpOptions>().Endpoints!.Chains;
+
+        var tenanted = chains.Single(x => x.Method.Method.Name == nameof(Gh4516Endpoint.Tenanted));
+        tenanted.BuildEndpoint(RouteWarmup.Lazy)
+            .Metadata.OfType<IProducesResponseTypeMetadata>()
+            .Select(x => x.StatusCode)
+            .ShouldContain(404);
+
+        // ...and a chain that resolves no tenant cannot fail to resolve one, so it must not advertise it
+        var notTenanted = chains.Single(x => x.Method.Method.Name == nameof(Gh4516Endpoint.NotTenanted));
+        notTenanted.BuildEndpoint(RouteWarmup.Lazy)
+            .Metadata.OfType<IProducesResponseTypeMetadata>()
+            .Select(x => x.StatusCode)
+            .ShouldNotContain(404);
+    }
+
+    private static async Task<IAlbaHost> startHostAsync(bool mapUnknownTenant)
+    {
+        var builder = WebApplication.CreateBuilder([]);
+
+        builder.Host.UseWolverine(opts =>
+        {
+            opts.Durability.Mode = DurabilityMode.MediatorOnly;
+            opts.Discovery.DisableConventionalDiscovery();
+            opts.Discovery.IncludeAssembly(typeof(unknown_tenant_problem_details_4516).Assembly);
+        });
+
+        builder.Services.AddWolverineHttp();
+
+        return await AlbaHost.For(builder, app => app.MapWolverineEndpoints(opts =>
+        {
+            opts.TenantId.IsQueryStringValue("tenantId");
+
+            // Without AssertExists(), a missing tenant id binds *DEFAULT* and reaches the handler --
+            // which is what the a_missing_tenant_id_still_answers_400_not_404 test is guarding.
+            opts.TenantId.AssertExists();
+
+            if (mapUnknownTenant)
+            {
+                opts.MapUnknownTenantToNotFound();
+            }
+
+            opts.CustomizeHttpEndpointDiscovery(q =>
+                q.Excludes.WithCondition("Not the GH-4516 endpoint", type => type != typeof(Gh4516Endpoint)));
+        }));
+    }
+}
+
+public static class Gh4516Endpoint
+{
+    // Stands in for a store or tenant source that does not know the id. Anything that resolves a tenant
+    // -- Marten's database source, Wolverine's own tenant sources -- throws this same exception.
+    [RequiresTenant]
+    [WolverineGet("/gh4516/tenanted")]
+    public static string Tenanted(IMessageBus bus)
+    {
+        if (bus.TenantId != "acme")
+        {
+            throw new UnknownTenantIdException(bus.TenantId!);
+        }
+
+        return bus.TenantId!;
+    }
+
+    [NotTenanted]
+    [WolverineGet("/gh4516/anonymous")]
+    public static string NotTenanted() => "no tenant here";
+}

src/Http/Wolverine.Http/Runtime/TenancyProblemMapping.cs

@@ -0,0 +1,85 @@
+using JasperFx;
+using JasperFx.CodeGeneration;
+using JasperFx.CodeGeneration.Model;
+using JasperFx.MultiTenancy;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
+
+namespace Wolverine.Http.Runtime;
+
+/// <summary>
+/// GH-4516. Maps an <b>unknown</b> tenant id onto a 404 ProblemDetails instead of letting it escape as a 500.
+///
+/// <para>
+/// A <b>missing</b> mandatory tenant id is already handled: <c>[RequiresTenant]</c> stops the request with a
+/// 400 ProblemDetails through <c>HttpHandler.WriteTenantIdNotFound</c>. But a tenant id that is present on
+/// the request and simply has no database or registration behind it throws
+/// <see cref="UnknownTenantIdException"/> from the store or from Wolverine's own tenant sources, which
+/// Wolverine.Http did not catch -- so the client got a 500 for what is a client-side error.
+/// </para>
+///
+/// <para>
+/// 404 rather than 400 on purpose: it reads as "the thing you addressed does not exist", and it keeps 400
+/// meaning "you did not say which tenant", which is the distinction the two failures actually have.
+/// </para>
+/// </summary>
+public static class UnknownTenantMiddleware
+{
+    public static ProblemDetails OnException(UnknownTenantIdException ex)
+    {
+        return TenancyProblemMapping.ToUnknownTenantProblemDetails(ex);
+    }
+}
+
+/// <summary>
+/// GH-4516. The one place the tenancy ProblemDetails are shaped, so they cannot drift from each other or
+/// from <see cref="ConflictMapping"/>.
+/// </summary>
+public static class TenancyProblemMapping
+{
+    public const int UnknownTenantStatusCode = StatusCodes.Status404NotFound;
+
+    public static ProblemDetails ToUnknownTenantProblemDetails(Exception ex)
+    {
+        return new ProblemDetails
+        {
+            Status = UnknownTenantStatusCode,
+            Title = "Unknown tenant",
+            Detail = ex.Message
+        };
+    }
+
+    /// <summary>
+    /// The default set of chains this mapping applies to: the ones that can actually resolve a tenant, and
+    /// so are the only ones that can fail to. A chain that is explicitly
+    /// <see cref="Wolverine.Http.TenancyMode.None"/> -- or that never declared a tenancy mode at all --
+    /// has nothing to report.
+    /// </summary>
+    public static bool IsTenanted(HttpChain chain)
+    {
+        return chain.TenancyMode is TenancyMode.Required or TenancyMode.Maybe;
+    }
+}
+
+/// <summary>
+/// GH-4516. Stamps <c>ProducesProblem(404)</c> on the tenanted chains, so the OpenAPI document advertises
+/// the unknown-tenant response the same way <see cref="ConflictProblemPolicy"/> advertises the conflict.
+/// </summary>
+public class UnknownTenantProblemPolicy : IHttpPolicy
+{
+    private readonly Func<HttpChain, bool> _filter;
+
+    public UnknownTenantProblemPolicy(Func<HttpChain, bool> filter)
+    {
+        _filter = filter;
+    }
+
+    public void Apply(IReadOnlyList<HttpChain> chains, GenerationRules rules, IServiceContainer container)
+    {
+        foreach (var chain in chains.Where(_filter))
+        {
+            chain.Metadata.ProducesProblem(TenancyProblemMapping.UnknownTenantStatusCode);
+        }
+    }
+}

src/Http/Wolverine.Http/WolverineHttpOptions.cs

@@ -481,6 +481,42 @@ public void MapConcurrencyFailuresToConflict(Func<HttpChain, bool>? filter = nul
         Policies.Add(new ConflictProblemPolicy(filter));
     }
 
+    /// <summary>
+    /// GH-4516. Map an <b>unknown</b> tenant id onto a 404 ProblemDetails instead of letting it escape as an
+    /// unhandled 500, and advertise the 404 in the OpenAPI document.
+    ///
+    /// <para>
+    /// A <b>missing</b> mandatory tenant id is already handled without this: <c>[RequiresTenant]</c> stops
+    /// the request with a 400 ProblemDetails. This covers the other case -- a tenant id that is present on
+    /// the request and simply has no database or registration behind it, which throws
+    /// <see cref="JasperFx.MultiTenancy.UnknownTenantIdException"/> from the store or from Wolverine's own
+    /// tenant sources. That is a client-side error, and it was answering 500.
+    /// </para>
+    ///
+    /// <para>
+    /// 404 rather than 400 on purpose: it reads as "the thing you addressed does not exist", and it keeps
+    /// 400 meaning "you did not say which tenant".
+    /// </para>
+    ///
+    /// <para>
+    /// A <i>disabled</i> tenant would deserve a 403 -- the tenant exists and access is refused -- but on
+    /// Marten and Polecat it is indistinguishable from unknown today, and JasperFx has no lifted
+    /// <c>DisabledTenantException</c> to catch. When it lands, it belongs here.
+    /// </para>
+    /// </summary>
+    /// <param name="filter">
+    /// Which chains the mapping applies to. Defaults to the tenanted chains -- those declared
+    /// <c>[RequiresTenant]</c> or <c>[MaybeTenanted]</c> -- since a chain that resolves no tenant cannot
+    /// fail to resolve one.
+    /// </param>
+    public void MapUnknownTenantToNotFound(Func<HttpChain, bool>? filter = null)
+    {
+        filter ??= TenancyProblemMapping.IsTenanted;
+
+        AddMiddleware(typeof(UnknownTenantMiddleware), filter);
+        Policies.Add(new UnknownTenantProblemPolicy(filter));
+    }
+
     /// <summary>
     ///     Add a new IResourceWriterPolicy for the Wolverine endpoints
     /// </summary>