refactor: Remove duplicate router-level exception helpers
All routers now let domain exceptions propagate to the global handlers in main.py instead of catching and converting them to HTTPException. This eliminates: - Duplicate exception-to-HTTP-status mappings across 8 routers - Duplicate helper functions (_bad_gateway, _not_found, _conflict, etc.) - Inconsistent error response formats Changes: - Removed all try/except blocks from routers that catch domain exceptions - Removed duplicate helper functions from all routers - Added missing exception handlers to main.py for: * ActionNameError * FilterNameError * JailNameError * JailNotFoundInConfigError * FilterInvalidRegexError - Removed unused imports from affected routers All domain exceptions now propagate to the single authoritative mapping in main.py, ensuring consistent error codes, messages, and logging across the API. Affected routers: - action_config.py: Removed _action_not_found, _bad_request, _not_found helpers - bans.py: Removed try/except in ban/unban endpoints - config_misc.py: Removed try/except blocks - file_config.py: Removed 6 try/except blocks and _service_unavailable helper - filter_config.py: Removed try/except blocks - geo.py: Removed try/except in lookup_ip endpoint - jail_config.py: Removed try/except blocks - jails.py: Removed try/except blocks - server.py: Removed try/except blocks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -57,48 +57,6 @@ router: APIRouter = APIRouter(prefix="/jails", tags=["Jail Config"])
|
||||
|
||||
_NamePath = Annotated[str, Path(description='Jail name as configured in fail2ban.')]
|
||||
|
||||
|
||||
def _not_found(name: str) -> HTTPException:
|
||||
return HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Jail not found: {name!r}",
|
||||
)
|
||||
|
||||
|
||||
def _bad_gateway(exc: Exception) -> HTTPException:
|
||||
return HTTPException(
|
||||
status_code=status.HTTP_502_BAD_GATEWAY,
|
||||
detail=f"Cannot reach fail2ban: {exc}",
|
||||
)
|
||||
|
||||
|
||||
def _unprocessable(message: str) -> HTTPException:
|
||||
return HTTPException(
|
||||
status_code=status.HTTP_422_UNPROCESSABLE_CONTENT,
|
||||
detail=message,
|
||||
)
|
||||
|
||||
|
||||
def _bad_request(message: str) -> HTTPException:
|
||||
return HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=message,
|
||||
)
|
||||
|
||||
|
||||
def _filter_not_found(name: str) -> HTTPException:
|
||||
return HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Filter not found: {name!r}",
|
||||
)
|
||||
|
||||
|
||||
def _action_not_found(name: str) -> HTTPException:
|
||||
return HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Action not found: {name!r}",
|
||||
)
|
||||
|
||||
@router.get(
|
||||
"",
|
||||
response_model=JailConfigListResponse,
|
||||
@@ -121,10 +79,7 @@ async def get_jail_configs(
|
||||
Returns:
|
||||
:class:`~app.models.config.JailConfigListResponse`.
|
||||
"""
|
||||
try:
|
||||
return await config_service.list_jail_configs(socket_path)
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
return await config_service.list_jail_configs(socket_path)
|
||||
|
||||
|
||||
|
||||
@@ -206,12 +161,7 @@ async def get_jail_config(
|
||||
HTTPException: 404 when the jail does not exist.
|
||||
HTTPException: 502 when fail2ban is unreachable.
|
||||
"""
|
||||
try:
|
||||
return await config_service.get_jail_config(socket_path, name)
|
||||
except JailNotFoundError:
|
||||
raise _not_found(name) from None
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
return await config_service.get_jail_config(socket_path, name)
|
||||
|
||||
|
||||
|
||||
@@ -245,16 +195,7 @@ async def update_jail_config(
|
||||
HTTPException: 400 when a set command is rejected.
|
||||
HTTPException: 502 when fail2ban is unreachable.
|
||||
"""
|
||||
try:
|
||||
await config_service.update_jail_config(socket_path, name, body)
|
||||
except JailNotFoundError:
|
||||
raise _not_found(name) from None
|
||||
except ConfigValidationError as exc:
|
||||
raise _unprocessable(str(exc)) from exc
|
||||
except ConfigOperationError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
await config_service.update_jail_config(socket_path, name, body)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -292,14 +233,7 @@ async def add_log_path(
|
||||
HTTPException: 400 when the command is rejected.
|
||||
HTTPException: 502 when fail2ban is unreachable.
|
||||
"""
|
||||
try:
|
||||
await config_service.add_log_path(socket_path, name, body)
|
||||
except JailNotFoundError:
|
||||
raise _not_found(name) from None
|
||||
except ConfigOperationError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
await config_service.add_log_path(socket_path, name, body)
|
||||
|
||||
|
||||
|
||||
@@ -332,14 +266,7 @@ async def delete_log_path(
|
||||
HTTPException: 400 when the command is rejected.
|
||||
HTTPException: 502 when fail2ban is unreachable.
|
||||
"""
|
||||
try:
|
||||
await config_service.delete_log_path(socket_path, name, log_path)
|
||||
except JailNotFoundError:
|
||||
raise _not_found(name) from None
|
||||
except ConfigOperationError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
await config_service.delete_log_path(socket_path, name, log_path)
|
||||
|
||||
|
||||
|
||||
@@ -381,24 +308,7 @@ async def activate_jail(
|
||||
"""
|
||||
req = body if body is not None else ActivateJailRequest()
|
||||
|
||||
try:
|
||||
result = await jail_config_service.activate_jail(config_dir, socket_path, name, req)
|
||||
except JailNameError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except JailNotFoundInConfigError:
|
||||
raise _not_found(name) from None
|
||||
except JailAlreadyActiveError:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail=f"Jail {name!r} is already active.",
|
||||
) from None
|
||||
except ConfigWriteError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to write config override: {exc}",
|
||||
) from exc
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
result = await jail_config_service.activate_jail(config_dir, socket_path, name, req)
|
||||
|
||||
if result.active:
|
||||
record_activation(app, name)
|
||||
@@ -438,25 +348,7 @@ async def deactivate_jail(
|
||||
HTTPException: 502 if fail2ban is unreachable.
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await jail_config_service.deactivate_jail(config_dir, socket_path, name)
|
||||
except JailNameError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except JailNotFoundInConfigError:
|
||||
raise _not_found(name) from None
|
||||
except JailAlreadyInactiveError:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail=f"Jail {name!r} is already inactive.",
|
||||
) from None
|
||||
except ConfigWriteError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to write config override: {exc}",
|
||||
) from exc
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
|
||||
result = await jail_config_service.deactivate_jail(config_dir, socket_path, name)
|
||||
return result
|
||||
|
||||
|
||||
@@ -494,24 +386,7 @@ async def delete_jail_local_override(
|
||||
HTTPException: 502 if fail2ban is unreachable.
|
||||
"""
|
||||
|
||||
try:
|
||||
await jail_config_service.delete_jail_local_override(config_dir, socket_path, name)
|
||||
except JailNameError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except JailNotFoundInConfigError:
|
||||
raise _not_found(name) from None
|
||||
except JailAlreadyActiveError:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail=f"Jail {name!r} is currently active; deactivate it first.",
|
||||
) from None
|
||||
except ConfigWriteError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to delete config override: {exc}",
|
||||
) from exc
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
await jail_config_service.delete_jail_local_override(config_dir, socket_path, name)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -549,10 +424,7 @@ async def validate_jail(
|
||||
HTTPException: 400 if *name* contains invalid characters.
|
||||
HTTPException: 404 if *name* is not found in any config file.
|
||||
"""
|
||||
try:
|
||||
return await jail_config_service.validate_jail_config(config_dir, name)
|
||||
except JailNameError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
return await jail_config_service.validate_jail_config(config_dir, name)
|
||||
|
||||
|
||||
@router.post(
|
||||
@@ -590,15 +462,7 @@ async def rollback_jail(
|
||||
"""
|
||||
start_cmd_parts: list[str] = start_cmd.split()
|
||||
|
||||
try:
|
||||
result = await jail_config_service.rollback_jail(config_dir, socket_path, name, start_cmd_parts)
|
||||
except JailNameError as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except ConfigWriteError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to write config override: {exc}",
|
||||
) from exc
|
||||
result = await jail_config_service.rollback_jail(config_dir, socket_path, name, start_cmd_parts)
|
||||
|
||||
if result.fail2ban_running:
|
||||
clear_pending_recovery(app)
|
||||
@@ -638,22 +502,7 @@ async def assign_filter_to_jail(
|
||||
HTTPException: 404 if the jail or filter does not exist.
|
||||
HTTPException: 500 if writing fails.
|
||||
"""
|
||||
try:
|
||||
await filter_config_service.assign_filter_to_jail(config_dir, socket_path, name, body, do_reload=reload)
|
||||
except (JailNameError, FilterNameError) as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except JailNotFoundInConfigError:
|
||||
raise _not_found(name) from None
|
||||
except FilterNotFoundError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Filter not found: {exc.name!r}",
|
||||
) from exc
|
||||
except ConfigWriteError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to write jail override: {exc}",
|
||||
) from exc
|
||||
await filter_config_service.assign_filter_to_jail(config_dir, socket_path, name, body, do_reload=reload)
|
||||
|
||||
|
||||
@router.post(
|
||||
@@ -688,22 +537,7 @@ async def assign_action_to_jail(
|
||||
HTTPException: 404 if the jail or action does not exist.
|
||||
HTTPException: 500 if writing fails.
|
||||
"""
|
||||
try:
|
||||
await action_config_service.assign_action_to_jail(config_dir, socket_path, name, body, do_reload=reload)
|
||||
except (JailNameError, ActionNameError) as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except JailNotFoundInConfigError:
|
||||
raise _not_found(name) from None
|
||||
except ActionNotFoundError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Action not found: {exc.name!r}",
|
||||
) from exc
|
||||
except ConfigWriteError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to write jail override: {exc}",
|
||||
) from exc
|
||||
await action_config_service.assign_action_to_jail(config_dir, socket_path, name, body, do_reload=reload)
|
||||
|
||||
|
||||
@router.delete(
|
||||
@@ -737,23 +571,13 @@ async def remove_action_from_jail(
|
||||
HTTPException: 404 if the jail is not found in config files.
|
||||
HTTPException: 500 if writing fails.
|
||||
"""
|
||||
try:
|
||||
await action_config_service.remove_action_from_jail(
|
||||
config_dir,
|
||||
socket_path,
|
||||
name,
|
||||
action_name,
|
||||
do_reload=reload,
|
||||
)
|
||||
except (JailNameError, ActionNameError) as exc:
|
||||
raise _bad_request(str(exc)) from exc
|
||||
except JailNotFoundInConfigError:
|
||||
raise _not_found(name) from None
|
||||
except ConfigWriteError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Failed to write jail override: {exc}",
|
||||
) from exc
|
||||
await action_config_service.remove_action_from_jail(
|
||||
config_dir,
|
||||
socket_path,
|
||||
name,
|
||||
action_name,
|
||||
do_reload=reload,
|
||||
)
|
||||
# ---------------------------------------------------------------------------
|
||||
# Filter discovery endpoints (Task 2.1)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user