refactor(exceptions): split the tenancy errors into one module per domain - #1672
Merged
Merged
Conversation
peteski22
had a problem deploying
to
integration-tests
September 24, 2026 15:52 — with 
GitHub Actions
Error
GitHub Actions
Error
peteski22
had a problem deploying
to
integration-tests
September 24, 2026 15:52 — with
GitHub Actions
Error
GitHub Actions
Error
peteski22
had a problem deploying
to
integration-tests
September 24, 2026 15:52 — with
GitHub Actions
Error
GitHub Actions
Error
peteski22
had a problem deploying
to
integration-tests
September 24, 2026 15:52 — with
GitHub Actions
Error
GitHub Actions
Error
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: mozilla-ai/otari/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (68)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tbille
approved these changes
Sep 24, 2026
The five status-carrying bases are defined in `exceptions/_base.py` and reach callers through the `gateway.exceptions` package root. `services/tenancy/errors.py` re-exported them, so eleven modules named the tenancy module for a class it does not define. Splitting that module by domain (#1193) leaves no module there to name, and the package root also avoids running `services/tenancy/__init__.py`, which pulls in the tenancy service tree for a base class. No error class moves and no status changes.
…s.py `SecretBoxUnavailableTenancyError` reports an absent `OTARI_SECRET_KEY`, which is a deployment's condition rather than one domain's: provider keys, workspace MCP servers and both guardrail surfaces raise it. Splitting the tenancy errors by domain (#1193) leaves it no domain module, and putting it in one would make the other three import that domain for it. Pure move: the class, its status and its message are unchanged. Its docstring no longer points at the old module's docstring for why the class carries a status.
The three organization pricing errors sat in the tenancy error module because it was the only module whose classes the registered handler renders (#1193). `exceptions/` is now that home, so they move to their own domain's module. Pure move: the classes, their statuses and their messages are unchanged. The comment explaining why pricing errors lived under tenancy is dropped, since it no longer describes where they are.
…eptions.py The ten provider-key errors join the offered-model errors already in the providers module (#1193), key errors first, since a model is offered on a key. The module gains an `__all__`, which the split would otherwise drop for the ten names the tenancy module listed. Pure move: the classes, their statuses and their messages are unchanged.
Workspace MCP servers, workspace web search and the sandbox code-execution policy are all the tools domain, so their seven errors leave the tenancy module for their own (#1193). Pure move: the classes, their statuses and their messages are unchanged.
…ptions.py The twenty errors of the two organization guardrail surfaces, mandates and definitions, leave the tenancy module for their own domain's (#1193). The route docstrings that pointed at the tenancy module for where a status comes from now name the domain module, pricing's included. Pure move: the classes, their statuses and their messages are unchanged.
…ns.py Passwords, passkeys, OAuth, email verification and reset, the profile, and deployment-wide account administration are the identity domain, so their thirty errors leave the tenancy module for their own (#1193). Pure move: the classes, their statuses and their messages are unchanged. One cross-reference in `deployment_user_service` names the new module.
…s_exceptions.py The last thirty-one errors, organizations, workspaces, members, invitations, email-domain claims and first-boot provisioning, leave for their own domain module, and `services/tenancy/errors.py` goes with them (#1193). The design note it carried, that a class names its own status and one handler renders the family, moves to the `gateway.exceptions` package docstring, which is where every caller now reaches the family. The three remaining pointers at the old module name the package instead. `test_tenancy_errors_home.py` asserted that the bases the old module re-exported were the ones in `gateway.exceptions`. With no re-export left there is nothing for it to compare. Pure move: the classes, their statuses and their messages are unchanged.
The errors split (#1193) gives identity, organizations, guardrails, pricing and tools a module each, and the shared set one for the errors no single domain owns. `tenancy/errors.py` leaves the organizations services list. The service-module counts in "The shape today" are left as they are. Their recorded figures do not reproduce from the tree at the commit they name, so decrementing one for the removed module would record a guess.
peteski22
force-pushed
the
refactor/1193-errors-per-domain
branch
from
September 24, 2026 15:53
362519a to
6d73aae
Compare
This branch was successfully deployed
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
2 participants
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.
Description
Every error the gateway's tenancy, identity, pricing, provider-key, tools and guardrail surfaces can raise lived in one 1,353-line file, so almost any feature touching those areas edited it: 35 commits from 7 authors in the last 90 days. This puts each error class in the exceptions module of the domain it belongs to, and removes the old file.
Nothing changes for someone running Otari. Every error keeps its class name, its message and the HTTP status it renders as, so the API answers byte for byte what it answered before. The OpenAPI spec, the Postman collection and the dashboard client are all unchanged.
The one class that is not a single domain's is
SecretBoxUnavailableTenancyError, which reports an absentOTARI_SECRET_KEY. Provider keys, workspace MCP servers and both guardrail surfaces raise it, so it goes to a newexceptions/shared_exceptions.pyrather than into one of those domains, which would have made the other two import that domain for it.The issue asked for a
services/tenancy/errors.pyshim that re-exported for one release. That line predates #1255, which splitmodels/entities.pyand deleted the old module instead of shimming it. The same reasoning applies here and the file is deleted:gatewayis a private import package rather than a published API, a second import path for every error means new code can pick either and the split never finishes, and the downstream cost turns out to be small (see below).Open PRs that add or move an error class will conflict with this. #1659 is the only open PR that touches the old file today. The update is mechanical; see "Updating an open PR" below.
Downstream: otari-ai. The overlay names the old module in seven files. Six of them import only the status-carrying base classes, which already live in
gateway.exceptions, so their fix is one line each and it removes the import side-effect their own docstrings complain about ("importinggateway.services.tenancy.errorsruns its package__init__, which pulls otari's model tree and collides with the platform's onuser"). The seventh is a test importingNotAuthorizedError. A matching change has to land with the otari pin bump to the first release that contains this PR. Nothing breaks before that bump, because the pin does not move on its own.Updating an open PR
src/gateway/exceptions/<domain>_exceptions.py. The five status-carrying bases come fromgateway.exceptions, as they already did.from gateway.services.tenancy.errors import Xbecomesfrom gateway.exceptions.<domain>_exceptions import X. The table below says which module each class went to.exceptions/shared_exceptions.py.identity_exceptions.pyorganizations_exceptions.pyguardrails_exceptions.pyproviders_exceptions.pytools_exceptions.pypricing_exceptions.pyshared_exceptions.pyMeasures
services/tenancy/errors.pyerrors.py, 1,353 linesidentity_exceptions.py, 461 linesexceptions/shared_exceptions.pyThe issue's own measure, that
errors.pyleaves the 90-day churn list, can only be read once 90 days have passed.How to test it locally
The API-response criterion is covered by the existing integration suite: 521 of its tests exercise routes that raise a moved error, and they are unchanged apart from their import lines.
What was checked, on every commit as well as the tip:
make lint,make typecheck,make openapi-checkandmake postman-checkpass. The spec and the collection do not move, which is the evidence that no response body changed.mainfinds all 102 classes again underexceptions/, with identical source text. The single exception isSecretBoxUnavailableTenancyError, whose docstring pointed a reader at the deleted module's docstring for why the class carries a status; it now states the reason itself. That is called out in its commit.status_codeas before.Failing locally on this branch and on
mainalike, none related to this change: 20 unit tests around the master key, dashboard sessions and the hook CLI, plustest_config_env_loadingand the OSS smoke gate, all from a local environment gap; andtest_mcp_dependency_ceiling, which needs network egress, asAGENTS.mdrecords. Each was re-run onmainto confirm. One further hook-CLI timing test fails only under full-suite load and passes on its own; the hook CLI cannot import the gateway.PR Type
Relevant issues
Fixes #1193
Part of #1171
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).ARCHITECTURE.mdorscripts/check_architecture.py, the description names the rule and says why.No tests were added: behavior does not change and the existing suites cover it. Test files changed only their import lines. One test was deleted,
tests/unit/test_tenancy_errors_home.py, which asserted that the bases the old module re-exported were the ones ingateway.exceptions; with no re-export left there is nothing for it to compare. No rule inARCHITECTURE.mdorscripts/check_architecture.pychanges: this is the layout both already describe, and the architecture check passes unchanged.docs/domains.mdgains each domain's exceptions module. Its service-module counts are left alone: the recorded figures do not reproduce from the tree at the commit they name, so decrementing one for the removed module would record a guess. Separately, the feedback domain added by #1655 has no section in that page yet, which is not this PR's to add.AI Usage
AI Model/Tool used:
An AI coding agent, run by the author.
Any additional AI details you'd like to share:
The author made the decisions: deleting the old module rather than keeping a re-export shim, after weighing it against what the issue originally asked for and what the overlay actually imports; a shared module for the one error no domain owns, rather than
_base.pyor the module of whichever domain raises it most; and doing every domain on one branch with a commit each, as #1255 did, rather than a branch per domain. The agent did the moves and the import rewrites, wrote the AST and runtime comparisons againstmain, and drafted this description.NOTE:
When responding to reviewer questions, please respond yourself rather than copy/pasting reviewer comments into an AI and pasting back its answer. We want to discuss with you, not your AI :)
Summary
Moved tenancy errors into domain-specific exception modules under
gateway.exceptionsand updated gateway services, routes, and tests to use the new locations. Removedservices/tenancy/errors.pywithout adding a compatibility shim.This groups errors by domain and removes the old import path. The changes describe the error and response behavior as unchanged.