GH-4516: map an unknown tenant id to a 404 ProblemDetails instead of a 500 - #4551
Merged
Merged
Conversation
…a 500
There are two different tenancy failures on an HTTP request, and only one of them was
handled. A MISSING mandatory tenant id already stops the request with a 400
ProblemDetails through [RequiresTenant] / TenantId.AssertExists(). An UNKNOWN tenant id --
present on the request, but with no database or registration behind it -- throws
JasperFx.MultiTenancy.UnknownTenantIdException from the store or from Wolverine's own
tenant sources, which Wolverine.Http did not catch. The client got a 500 for what is a
client side error.
opts.MapUnknownTenantToNotFound();
maps it to a 404 ProblemDetails titled "Unknown tenant" and stamps ProducesProblem(404) so
the OpenAPI document advertises it. Built on the same shape as #4512's conflict mapping:
middleware plus an IHttpPolicy, one call, and the same optional chain predicate.
404 rather than 400 on purpose, as the issue proposes: it reads as "the thing you
addressed does not exist", and it 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 -- and there is a test asserting the missing-id case still answers
400.
Defaults to the tenanted chains -- those declared [RequiresTenant] or [MaybeTenanted] --
since a chain that resolves no tenant cannot fail to resolve one. A test asserts the 404
is NOT advertised on a [NotTenanted] chain.
DisabledTenantException is deliberately absent. The issue anticipated this: a disabled
tenant deserves a 403 because the tenant exists and access is refused, but it reads as
unknown on Marten and Polecat today and JasperFx has no lifted DisabledTenantException to
catch. Recorded in the XML docs on MapUnknownTenantToNotFound so it can be added in one
place when it lands.
Five new tests, including a negative control -- without the opt in, the exception still
escapes -- so the test proves the opt in is what changes the answer rather than something
else in the pipeline.
Full Wolverine.Http.Tests suite: 1067 passed, 10 skipped, 0 failed.
Closes #4516
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
jeremydmiller
added a commit
that referenced
this pull request
Sep 23, 2026
`unknown_tenant_problem_details_4516.without_the_opt_in_the_unknown_tenant_still_escapes` went red on CI the moment #4551 merged, and it takes the whole `test` job with it -- so every open PR is red regardless of its contents (#4553, #4557, #4558 at the time of writing, two of them from contributors). The test was self contradictory. It asked Alba to assert a 404 inside a scenario it expected to THROW: await Should.ThrowAsync<UnknownTenantIdException>(async () => await host.Scenario(x => { x.Get.Url("/gh4516/tenanted?tenantId=ghost"); x.StatusCodeShouldBe(404); // <- asserted on a request expected to blow up })); Locally the UnknownTenantIdException propagated before Alba evaluated its assertions, so the expected exception won and the test passed. On CI the host turned the exception into a 500 first, so Alba's own 404 assertion fired and raised ScenarioAssertionException instead of UnknownTenantIdException -- a different type, so Should.ThrowAsync failed. The control's actual purpose is narrower than what it asserted: it exists to show that MapUnknownTenantToNotFound() is what produces the 404, not something else in the pipeline. It is not a claim about HOW an unmapped failure surfaces, and that is exactly the part that varies by environment. It now uses IgnoreStatusCode() and asserts only that the response is not a mapped 404, tolerating the other surfacing -- the exception escaping the scenario -- and nothing else. Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4516. Builds on the API from #4512.
The gap
There are two different tenancy failures on an HTTP request, and only one was handled:
[RequiresTenant]/TenantId.AssertExists()An unknown tenant id throws
JasperFx.MultiTenancy.UnknownTenantIdExceptionfrom the store or from Wolverine's own tenant sources, which Wolverine.Http never caught — so the client got a 500 for what is a client-side error.maps it to a 404
ProblemDetailstitled Unknown tenant and stampsProducesProblem(404)so OpenAPI advertises it. Same shape as #4512's conflict mapping — middleware plus anIHttpPolicy, one call, same optional chain predicate.404, and the distinction it protects
404 rather than 400, as the issue proposes: it reads as "the thing you addressed does not exist", and it 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 — so there's a test asserting the missing-id case still answers 400 with the mapping turned on.
Defaults to the tenanted chains (
[RequiresTenant]/[MaybeTenanted]), since a chain that resolves no tenant cannot fail to resolve one. A test asserts the 404 is not advertised on a[NotTenanted]chain.DisabledTenantExceptionis deliberately absentThe issue anticipated this. A disabled tenant deserves a 403 — the tenant exists and access is refused — but it reads as unknown on Marten and Polecat today and JasperFx has no lifted
DisabledTenantExceptionto catch. That's recorded in the XML docs onMapUnknownTenantToNotFoundso it can be added in one place when it lands.Testing
Five new tests, including a negative control: without the opt-in,
UnknownTenantIdExceptionstill escapes. That's what makes the other four mean something — they prove the opt-in is what changes the answer, rather than something else in the pipeline.The #4547 dedupe flake happened not to fire on this run.
Docs: a new Unknown Tenants as 404 section in
exception-handling.md, including the message-handler equivalent (OnException<UnknownTenantIdException>().MoveToErrorQueue()— never retry, it's deterministic).🤖 Generated with Claude Code
https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj