Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
bf3e7ab
Hoist exception messages into a msg variable
anth-volk Oct 13, 2026
d945a53
Move SPM and worker validation raises out of try blocks
anth-volk Oct 13, 2026
76dca42
Give the v1 services typed exceptions and plain re-raises
anth-volk Oct 13, 2026
a1fbd36
Restructure exception handling in the Flask and FastAPI routes
anth-volk Oct 13, 2026
0205cf4
Raise TypeError for non-object bundle manifests in the version script
anth-volk Oct 13, 2026
61e4a01
Raise the forecast-year test error from a helper
anth-volk Oct 13, 2026
130cf4b
Decide each blind except: narrow one, keep the deliberate ones
anth-volk Oct 13, 2026
5c4e272
Enable ruff group 5 (exceptions and messages)
anth-volk Oct 13, 2026
b96d879
Name hoisted SPM error codes error_code, not msg
anth-volk Oct 13, 2026
0bac9f1
Put BLE001 suppression reasons in the " # reason" form
anth-volk Oct 13, 2026
5b28a55
Register TRY, EM and BLE in the ruff-lint rule
anth-volk Oct 13, 2026
c820c7e
Hoist the region filter messages that #3918 added into a msg variable
anth-volk Oct 14, 2026
d1d1cb3
Hoist the exception messages #3891 added into a msg variable
anth-volk Oct 15, 2026
e1897a8
Put the BLE001 suppression reasons #3891 added in the " # reason" form
anth-volk Oct 15, 2026
8f08c44
Use contextlib.suppress for the best-effort telemetry and logging calls
anth-volk Oct 19, 2026
a77d0bc
Catch only the expected errors in period parsing and the health probe
anth-volk Oct 19, 2026
a118c34
Raise TypeError for a non-object .py bundle manifest
anth-volk Oct 19, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions .github/find-api-model-versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@
def _manifest_versions(bundle: dict) -> dict[str, str]:
packages = bundle.get("packages")
if not isinstance(packages, dict):
raise RuntimeError("PolicyEngine bundle manifest has no package mapping.")
msg = "PolicyEngine bundle manifest has no package mapping."
raise TypeError(msg)

versions = {}
for package_name in REQUIRED_PACKAGES:
package = packages.get(package_name)
if not isinstance(package, dict) or not package.get("version"):
raise RuntimeError(
f"PolicyEngine bundle manifest has no version for {package_name}."
)
msg = f"PolicyEngine bundle manifest has no version for {package_name}."
raise RuntimeError(msg)
versions[package_name] = str(package["version"])
return versions

Expand All @@ -47,26 +47,27 @@ def _data_release_version(bundle: dict, country_id: str) -> str:
data_releases = bundle.get("data_releases")
release = data_releases.get(country_id) if isinstance(data_releases, dict) else None
if not isinstance(release, dict) or not release.get("version"):
raise RuntimeError(
f"PolicyEngine bundle manifest has no {country_id.upper()} data release."
)
msg = f"PolicyEngine bundle manifest has no {country_id.upper()} data release."
raise RuntimeError(msg)
return str(release["version"])


def find_api_model_versions() -> dict[str, str]:
"""Find and validate the package and data versions in the installed bundle."""
bundle = get_current_bundle()
if not isinstance(bundle, dict):
raise RuntimeError("Installed PolicyEngine bundle manifest is not an object.")
msg = "Installed PolicyEngine bundle manifest is not an object."
raise TypeError(msg)

manifest_versions = _manifest_versions(bundle)
for package_name, manifest_version in manifest_versions.items():
installed_version = distribution_version(package_name)
if installed_version != manifest_version:
raise RuntimeError(
msg = (
f"Installed {package_name} version {installed_version} does not match "
f"bundle manifest version {manifest_version}."
)
raise RuntimeError(msg)

expected_constants = {
"policyengine": POLICYENGINE_VERSION,
Expand All @@ -76,10 +77,11 @@ def find_api_model_versions() -> dict[str, str]:
}
for package_name, constant_version in expected_constants.items():
if constant_version != manifest_versions[package_name]:
raise RuntimeError(
msg = (
f"API version for {package_name} is {constant_version}, but the bundle "
f"manifest specifies {manifest_versions[package_name]}."
)
raise RuntimeError(msg)

return {
"POLICYENGINE_VERSION": manifest_versions["policyengine"],
Expand Down
17 changes: 10 additions & 7 deletions .github/scripts/verify_observability_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class VerificationProbe(NamedTuple):
def _authority(endpoint: str) -> str:
parsed = urlparse(endpoint)
if parsed.scheme != "https" or not parsed.hostname:
raise ValueError("collector endpoint must be an HTTPS URL")
msg = "collector endpoint must be an HTTPS URL"
raise ValueError(msg)
return f"{parsed.hostname}:{parsed.port or 443}"


Expand Down Expand Up @@ -193,7 +194,8 @@ def _raise_for_partial_rejection(
rejected = getattr(partial, rejected_field, 0)
if rejected:
message = getattr(partial, "error_message", "")
raise RuntimeError(f"collector rejected {rejected} {signal}: {message}")
msg = f"collector rejected {rejected} {signal}: {message}"
raise RuntimeError(msg)


def _get_json(url: str, access_token: str) -> dict[str, object] | None:
Expand All @@ -208,9 +210,8 @@ def _get_json(url: str, access_token: str) -> dict[str, object] | None:
if error.code == 404:
return None
detail = error.read().decode("utf-8", errors="replace")
raise RuntimeError(
f"Google Cloud verification request failed ({error.code}): {detail}"
) from error
msg = f"Google Cloud verification request failed ({error.code}): {detail}"
raise RuntimeError(msg) from error


def _trace_available(
Expand Down Expand Up @@ -302,7 +303,8 @@ def _wait_for_delivery(
)
if not found
)
raise RuntimeError(f"collector verification data was not stored: {missing}")
msg = f"collector verification data was not stored: {missing}"
raise RuntimeError(msg)
time.sleep(min(poll_seconds, remaining))


Expand Down Expand Up @@ -357,7 +359,8 @@ def verify(
if error.code() is not grpc.StatusCode.UNIMPLEMENTED:
raise
else:
raise RuntimeError("collector unexpectedly accepted application logs")
msg = "collector unexpectedly accepted application logs"
raise RuntimeError(msg)
_wait_for_delivery(
project_id=project_id,
trace_id=trace_id,
Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`.
- `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`)
- `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy)
- `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format)
- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, UP, W))
- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W))
- `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`)
- `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`)
- `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`)
Expand Down
1 change: 1 addition & 0 deletions changelog.d/ruff-exceptions.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enabled ruff's exception rules (TRY, EM, BLE): exception messages are built before the raise, raises inside try blocks moved into helpers, the v1 services raise typed exceptions, best-effort telemetry and logging calls use a commented contextlib.suppress block, and every other deliberate catch-all carries a reasoned suppression. Period parsing and the simulation health probe now catch only the errors they expect, and a non-object .py bundle manifest raises TypeError. Error responses and message text are unchanged, except that a non-HTTP failure in the simulation health probe now gets the route's existing 503 for a client failure.
8 changes: 4 additions & 4 deletions docs/generated/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ flowchart LR
| `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — |
| `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — |
| `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — |
| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, UP, W) | `pr.yml:lint` | — |
| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W) | `pr.yml:lint` | — |
| `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** |
| `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — |
| `api.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**` | `pr.yml:lint` | — |
Expand All @@ -292,7 +292,7 @@ flowchart LR
e_ruff_format["ruff format"]
j_CI_pr_yml_lint["CI pr.yml:lint"]
r_api_quality_ruff_lint("api.quality.ruff-lint (enforced)")
e_ruff_check__ANN__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_["ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, UP, W)"]
e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_["ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W)"]
r_api_quality_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)")
e_pytest_tests_unit_test_runtime_evaluated_annotations_py["pytest tests/unit/test_runtime_evaluated_annotations.py"]
j_CI_pr_yml_test["CI pr.yml:test"]
Expand All @@ -308,8 +308,8 @@ flowchart LR
e_mypy --> j_CI_pr_yml_typecheck_v2
r_api_quality_ruff_format --> e_ruff_format
e_ruff_format --> j_CI_pr_yml_lint
r_api_quality_ruff_lint --> e_ruff_check__ANN__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_
e_ruff_check__ANN__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_ --> j_CI_pr_yml_lint
r_api_quality_ruff_lint --> e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_
e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ --> j_CI_pr_yml_lint
r_api_quality_runtime_evaluated_models_resolve --> e_pytest_tests_unit_test_runtime_evaluated_annotations_py
e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test
r_api_quality_tests_pass --> e_CI_job_test
Expand Down
9 changes: 6 additions & 3 deletions migrations/v1/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,21 @@
provided_connection = config.attributes.get("connection")
if provided_connection is not None:
if provided_connection.dialect.name != "mysql":
raise RuntimeError("Alembic migrations must target a MySQL database")
msg = "Alembic migrations must target a MySQL database"
raise RuntimeError(msg)
else:
database_url = os.environ.get("ALEMBIC_DATABASE_URL") or config.get_main_option(
"sqlalchemy.url"
)
if not database_url:
raise RuntimeError(
msg = (
"ALEMBIC_DATABASE_URL is required; the local SQLite cache is not an "
"Alembic migration target"
)
raise RuntimeError(msg)
if make_url(database_url).get_backend_name() != "mysql":
raise RuntimeError("Alembic migrations must target a MySQL database")
msg = "Alembic migrations must target a MySQL database"
raise RuntimeError(msg)
config.set_main_option("sqlalchemy.url", database_url.replace("%", "%%"))

target_metadata = V1Base.metadata
Expand Down
3 changes: 2 additions & 1 deletion migrations/v2/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ def _configure(connection: Connection) -> None:

def run_migrations_offline() -> None:
"""Raise, because v2 migrations need a live connection to qualify the target."""
raise RuntimeError(
msg = (
"v2 migrations require an online connection so target identity and "
"generated application-data before/after states can be verified"
)
raise RuntimeError(msg)


def run_migrations_online() -> None:
Expand Down
35 changes: 15 additions & 20 deletions policyengine_api/asgi_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import contextlib
import time
from contextlib import asynccontextmanager
from typing import TYPE_CHECKING, Any
Expand Down Expand Up @@ -236,19 +237,19 @@ async def add_request_context_and_migration_logging(
request.state.policyengine_request_id = request_id
_asgi_request_id.reset(context_token)
context_token = _asgi_request_id.set(request_id)
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass
try:
# Telemetry must never fail the request.
with contextlib.suppress(Exception):
request_runtime.set_context(
request_id=request_id,
)
except Exception:
pass

def log_native_route(status_code: int) -> None:
if not native_request:
return
try:
# Migration logging must never fail the request.
with contextlib.suppress(Exception):
log_migration_request(
request_id=request_id,
method=request.method,
Expand All @@ -261,8 +262,6 @@ def log_native_route(status_code: int) -> None:
),
route_impl=RouteImplementation.FASTAPI_NATIVE,
)
except Exception:
pass

def finish_native_route(
status_code: int,
Expand All @@ -271,21 +270,18 @@ def finish_native_route(
if not native_request:
return
resolved_route = getattr(request.scope.get("route"), "path", initial_route)
try:
# Telemetry must never fail the request.
with contextlib.suppress(Exception):
request_runtime.update_request_route(resolved_route)
except Exception:
pass
try:
# Telemetry must never fail the request.
with contextlib.suppress(Exception):
request_runtime.update_request_status(status_code)
except Exception:
pass
try:
# Telemetry must never fail the request.
with contextlib.suppress(Exception):
request_runtime.end_request(
status_code=status_code,
error=error,
)
except Exception:
pass

try:
try:
Expand All @@ -301,20 +297,19 @@ def finish_native_route(
)
)
if native_request and response_observability_id is not None:
try:
# Telemetry must never fail the request.
with contextlib.suppress(Exception):
# Route-level spans have completed at this point, so this
# call applies the selected calculation identifier to the
# still-current server request span.
request_runtime.set_context(
observability_id=response_observability_id
)
except Exception:
pass
if native_request:
try:
for name, value in request_runtime.response_headers().items():
response.headers[name] = value
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass
_apply_request_id_header(response, request_id)
if response_observability_id is not None:
Expand Down
15 changes: 8 additions & 7 deletions policyengine_api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,14 @@ def _load_policyengine_bundle() -> dict | None:
with open(get_py_manifest()) as manifest_file:
manifest = json.load(manifest_file)
except Exception as exc:
raise RuntimeError(
msg = (
"Could not read PolicyEngine .py bundle manifest from the installed "
"policyengine wheel."
) from exc
)
raise RuntimeError(msg) from exc
if not isinstance(manifest, dict):
raise RuntimeError("PolicyEngine .py bundle manifest must be a JSON object.")
msg = "PolicyEngine .py bundle manifest must be a JSON object."
raise TypeError(msg)
return manifest


Expand Down Expand Up @@ -162,7 +164,7 @@ def get_bundle_default_dataset_option(country_id: str) -> dict:
country: _resolve_distribution_version(_dist_versions, package_name)
for country, package_name in zip(COUNTRIES, COUNTRY_PACKAGE_NAMES)
}
except Exception:
except Exception: # noqa: BLE001 # unreadable package metadata falls back to 0.0.0; import must not fail
_dist_versions = {}
COUNTRY_PACKAGE_VERSIONS = {country: "0.0.0" for country in COUNTRIES}

Expand Down Expand Up @@ -204,9 +206,8 @@ def _build_runtime_cache_version(
prefix = "e" if scope == "economy_impact" else "r"
digest_length = 10 - len(prefix) - len(schema_version)
if digest_length < 4:
raise ValueError(
f"Runtime cache version for {scope} does not fit in VARCHAR(10)"
)
msg = f"Runtime cache version for {scope} does not fit in VARCHAR(10)"
raise ValueError(msg)

raw = "|".join(
(
Expand Down
2 changes: 1 addition & 1 deletion policyengine_api/country.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ def _normalize_reform_values(
node_type = float
try:
value = float(value)
except Exception:
except (TypeError, ValueError, OverflowError):
pass
normalized_reform[parameter_name][time_period] = node_type(value)
return normalized_reform
Expand Down
11 changes: 6 additions & 5 deletions policyengine_api/data/places.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ def parse_place_code(place_code: str) -> tuple[str, str]:

"""
if "-" not in place_code:
raise ValueError(
msg = (
f"Invalid place format: '{place_code}'. "
"Expected format: STATE_ABBREV-PLACE_FIPS (e.g., NJ-57000)"
)
raise ValueError(msg)
return place_code.split("-", 1)


Expand All @@ -41,9 +42,9 @@ def validate_place_code(place_code: str) -> None:
state_abbrev, place_fips = parse_place_code(place_code)

if state_abbrev.lower() not in get_valid_state_codes():
raise ValueError(f"Invalid state in place code: '{state_abbrev}'")
msg = f"Invalid state in place code: '{state_abbrev}'"
raise ValueError(msg)

if not place_fips.isdigit() or len(place_fips) != 5:
raise ValueError(
f"Invalid FIPS code in place: '{place_fips}'. Expected 5-digit FIPS code"
)
msg = f"Invalid FIPS code in place: '{place_fips}'. Expected 5-digit FIPS code"
raise ValueError(msg)
Loading
Loading