From a2501d0e3e8e78fb5a13c45876c9a0f63fcd41f7 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Thu, 6 Jun 2024 16:49:08 -0700 Subject: [PATCH] Add Slack reporting of uncaught exceptions Add support for reporting uncaught exceptions in route handlers to Slack if a Slack incoming webhook is configured for that purpose. Add tests to verify that normal user errors don't trigger Slack exceptions. Tidy up development dependencies and add pytest-sugar. --- changelog.d/20240606_141501_rra_DM_44606.md | 3 ++ requirements/dev.in | 9 ++-- requirements/dev.txt | 60 ++++----------------- requirements/main.txt | 2 +- requirements/tox.txt | 34 +++--------- src/vocutouts/config.py | 16 ++++-- src/vocutouts/handlers/external.py | 21 ++++---- src/vocutouts/handlers/internal.py | 20 +++---- src/vocutouts/main.py | 24 ++++++--- src/vocutouts/uws/config.py | 15 +++--- src/vocutouts/uws/exceptions.py | 10 ++-- src/vocutouts/uws/handlers.py | 9 ++-- tests/conftest.py | 9 ++++ tests/handlers/async_test.py | 8 ++- tests/handlers/error_test.py | 60 +++++++++++++++++++-- tests/handlers/sync_test.py | 8 ++- tests/support/uws.py | 9 ++-- tests/uws/conftest.py | 12 +++++ tests/uws/errors_test.py | 8 ++- tests/uws/job_error_test.py | 25 +++++++-- tox.ini | 5 +- 21 files changed, 221 insertions(+), 146 deletions(-) create mode 100644 changelog.d/20240606_141501_rra_DM_44606.md diff --git a/changelog.d/20240606_141501_rra_DM_44606.md b/changelog.d/20240606_141501_rra_DM_44606.md new file mode 100644 index 0000000..0305361 --- /dev/null +++ b/changelog.d/20240606_141501_rra_DM_44606.md @@ -0,0 +1,3 @@ +### New features + +- Add support for sending Slack notifications for uncaught exceptions in route handlers. diff --git a/requirements/dev.in b/requirements/dev.in index 813d70b..814a6ad 100644 --- a/requirements/dev.in +++ b/requirements/dev.in @@ -7,12 +7,7 @@ -c main.txt -# Development -pre-commit -tox -tox-uv - -# Testing and linting +# Testing asgi-lifespan coverage[toml] httpx @@ -20,7 +15,9 @@ mypy pytest pytest-asyncio pytest-cov +pytest-sugar pytest-timeout +respx sqlalchemy[mypy] # Documentation diff --git a/requirements/dev.txt b/requirements/dev.txt index f2cf6f1..0245e5b 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -8,20 +8,12 @@ asgi-lifespan==2.1.0 # via -r requirements/dev.in attrs==23.2.0 # via scriv -cachetools==5.3.3 - # via - # -c requirements/main.txt - # tox certifi==2024.6.2 # via # -c requirements/main.txt # httpcore # httpx # requests -cfgv==3.4.0 - # via pre-commit -chardet==5.2.0 - # via tox charset-normalizer==3.3.2 # via # -c requirements/main.txt @@ -33,18 +25,10 @@ click==8.1.7 # scriv click-log==0.4.0 # via scriv -colorama==0.4.6 - # via tox coverage==7.5.3 # via # -r requirements/dev.in # pytest-cov -distlib==0.3.8 - # via virtualenv -filelock==3.14.0 - # via - # tox - # virtualenv greenlet==3.0.3 # via # -c requirements/main.txt @@ -61,8 +45,7 @@ httpx==0.27.0 # via # -c requirements/main.txt # -r requirements/dev.in -identify==2.5.36 - # via pre-commit + # respx idna==3.7 # via # -c requirements/main.txt @@ -93,47 +76,34 @@ mypy==1.10.0 # sqlalchemy mypy-extensions==1.0.0 # via mypy -nodeenv==1.9.1 - # via pre-commit packaging==24.0 # via # -c requirements/main.txt - # pyproject-api # pytest - # tox - # tox-uv -platformdirs==4.2.2 - # via - # tox - # virtualenv + # pytest-sugar pluggy==1.5.0 - # via - # pytest - # tox -pre-commit==3.7.1 - # via -r requirements/dev.in -pyproject-api==1.6.1 - # via tox + # via pytest pytest==8.2.2 # via # -r requirements/dev.in # pytest-asyncio # pytest-cov + # pytest-sugar # pytest-timeout pytest-asyncio==0.23.7 # via -r requirements/dev.in pytest-cov==5.0.0 # via -r requirements/dev.in +pytest-sugar==1.0.0 + # via -r requirements/dev.in pytest-timeout==2.3.1 # via -r requirements/dev.in -pyyaml==6.0.1 - # via - # -c requirements/main.txt - # pre-commit requests==2.32.3 # via # -c requirements/main.txt # scriv +respx==0.21.1 + # via -r requirements/dev.in scriv==1.5.1 # via -r requirements/dev.in sniffio==1.3.1 @@ -146,12 +116,8 @@ sqlalchemy==2.0.30 # via # -c requirements/main.txt # -r requirements/dev.in -tox==4.15.1 - # via - # -r requirements/dev.in - # tox-uv -tox-uv==1.9.0 - # via -r requirements/dev.in +termcolor==2.4.0 + # via pytest-sugar typing-extensions==4.12.1 # via # -c requirements/main.txt @@ -161,9 +127,3 @@ urllib3==2.2.1 # via # -c requirements/main.txt # requests -uv==0.2.8 - # via tox-uv -virtualenv==20.26.2 - # via - # pre-commit - # tox diff --git a/requirements/main.txt b/requirements/main.txt index 35226b3..c8e8b72 100644 --- a/requirements/main.txt +++ b/requirements/main.txt @@ -173,7 +173,7 @@ rich==13.7.1 # via typer rsa==4.9 # via google-auth -safir @ git+https://github.com/lsst-sqre/safir@9ce8b39d810df41cf028afe1c8befea5613951a8 +safir @ git+https://github.com/lsst-sqre/safir@53062ff688230856f89ce9a82b9ec3036dcb4beb # via -r requirements/main.in shellingham==1.5.4 # via typer diff --git a/requirements/tox.txt b/requirements/tox.txt index 4fa1971..788924b 100644 --- a/requirements/tox.txt +++ b/requirements/tox.txt @@ -2,7 +2,6 @@ # uv pip compile --output-file requirements/tox.txt requirements/tox.in cachetools==5.3.3 # via - # -c requirements/dev.txt # -c requirements/main.txt # tox certifi==2024.6.2 @@ -11,27 +10,20 @@ certifi==2024.6.2 # -c requirements/main.txt # requests chardet==5.2.0 - # via - # -c requirements/dev.txt - # tox + # via tox charset-normalizer==3.3.2 # via # -c requirements/dev.txt # -c requirements/main.txt # requests colorama==0.4.6 - # via - # -c requirements/dev.txt - # tox + # via tox distlib==0.3.8 - # via - # -c requirements/dev.txt - # virtualenv + # via virtualenv docker==7.1.0 # via tox-docker filelock==3.14.0 # via - # -c requirements/dev.txt # tox # virtualenv idna==3.7 @@ -48,7 +40,6 @@ packaging==24.0 # tox-uv platformdirs==4.2.2 # via - # -c requirements/dev.txt # tox # virtualenv pluggy==1.5.0 @@ -56,9 +47,7 @@ pluggy==1.5.0 # -c requirements/dev.txt # tox pyproject-api==1.6.1 - # via - # -c requirements/dev.txt - # tox + # via tox requests==2.32.3 # via # -c requirements/dev.txt @@ -66,27 +55,20 @@ requests==2.32.3 # docker tox==4.15.1 # via - # -c requirements/dev.txt # -r requirements/tox.in # tox-docker # tox-uv tox-docker==5.0.0 # via -r requirements/tox.in tox-uv==1.9.0 - # via - # -c requirements/dev.txt - # -r requirements/tox.in + # via -r requirements/tox.in urllib3==2.2.1 # via # -c requirements/dev.txt # -c requirements/main.txt # docker # requests -uv==0.2.8 - # via - # -c requirements/dev.txt - # tox-uv +uv==0.2.9 + # via tox-uv virtualenv==20.26.2 - # via - # -c requirements/dev.txt - # tox + # via tox diff --git a/src/vocutouts/config.py b/src/vocutouts/config.py index 379c392..6be89e9 100644 --- a/src/vocutouts/config.py +++ b/src/vocutouts/config.py @@ -83,12 +83,19 @@ class Config(BaseSettings): ), ) + slack_webhook: SecretStr | None = Field( + None, + title="Slack webhook for alerts", + description="If set, alerts will be posted to this Slack webhook", + ) + storage_url: str = Field( ..., title="Root URL for cutout results", description=( - "Must be an ``s3`` URL pointing to a Google Cloud Storage bucket" - " that is writable by the backend and readable by the frontend." + "Must be a ``gs`` or ``s3`` URL pointing to a Google Cloud Storage" + " bucket that is writable by the backend and readable by the" + " frontend." ), ) @@ -200,13 +207,14 @@ def arq_redis_settings(self) -> RedisSettings: def uws_config(self) -> UWSConfig: """Corresponding configuration for the UWS subsystem.""" return UWSConfig( + arq_mode=self.arq_mode, + arq_redis_settings=self.arq_redis_settings, execution_duration=self.timeout, lifetime=self.lifetime, database_url=self.database_url, database_password=self.database_password, - arq_mode=self.arq_mode, - arq_redis_settings=self.arq_redis_settings, signing_service_account=self.service_account, + slack_webhook=self.slack_webhook, ) diff --git a/src/vocutouts/handlers/external.py b/src/vocutouts/handlers/external.py index cb925af..a19ecb2 100644 --- a/src/vocutouts/handlers/external.py +++ b/src/vocutouts/handlers/external.py @@ -15,6 +15,7 @@ auth_logger_dependency, ) from safir.metadata import get_metadata +from safir.slack.webhook import SlackRouteErrorHandler from structlog.stdlib import BoundLogger from ..config import config @@ -27,9 +28,7 @@ from ..uws.handlers import uws_router from ..uws.models import ExecutionPhase, UWSJobParameter -__all__ = ["external_router"] - -external_router = APIRouter() +router = APIRouter(route_class=SlackRouteErrorHandler) """FastAPI router for all external handlers.""" _CAPABILITIES_TEMPLATE = """ @@ -61,8 +60,10 @@ """ +__all__ = ["router"] + -@external_router.get( +@router.get( "/", response_model=Index, response_model_exclude_none=True, @@ -87,7 +88,7 @@ async def get_index() -> Index: return Index(metadata=metadata) -@external_router.get( +@router.get( "/availability", description="VOSI-availability resource for the image cutout service", responses={200: {"content": {"application/xml": {}}}}, @@ -103,7 +104,7 @@ async def get_availability( return templates.availability(request, availability) -@external_router.get( +@router.get( "/capabilities", description="VOSI-capabilities resource for the image cutout service", responses={200: {"content": {"application/xml": {}}}}, @@ -177,7 +178,7 @@ async def _sync_request( return RedirectResponse(result.url, status_code=303) -@external_router.get( +@router.get( "/sync", description=( "Synchronously request a cutout. This will wait for the cutout to be" @@ -272,7 +273,7 @@ async def get_sync( ) -@external_router.post( +@router.post( "/sync", description=( "Synchronously request a cutout. This will wait for the cutout to be" @@ -473,7 +474,7 @@ async def create_job( return str(request.url_for("get_job", job_id=job.job_id)) -# Add the UWS routes to our external routes. This must be done after defining +# Add the UWS routes to our external routes. This must be done after defining # the POST handler for /jobs because of oddities in the implementation details # of include_router. -external_router.include_router(uws_router, prefix="/jobs") +router.include_router(uws_router, prefix="/jobs") diff --git a/src/vocutouts/handlers/internal.py b/src/vocutouts/handlers/internal.py index 7b498f4..a5e9ab9 100644 --- a/src/vocutouts/handlers/internal.py +++ b/src/vocutouts/handlers/internal.py @@ -1,8 +1,8 @@ """Internal HTTP handlers that serve relative to the root path, ``/``. These handlers aren't externally visible since the app is available at a path, -``/vocutouts``. See `vocutouts.handlers.external` for -the external endpoint handlers. +``/vocutouts``. See `vocutouts.handlers.external` for the external endpoint +handlers. These handlers should be used for monitoring, health checks, internal status, or other information that should not be visible outside the Kubernetes cluster. @@ -10,16 +10,17 @@ from fastapi import APIRouter from safir.metadata import Metadata, get_metadata +from safir.slack.webhook import SlackRouteErrorHandler from ..config import config -__all__ = ["get_index", "internal_router"] - -internal_router = APIRouter() +router = APIRouter(route_class=SlackRouteErrorHandler) """FastAPI router for all internal handlers.""" +__all__ = ["router"] + -@internal_router.get( +@router.get( "/", description=( "Return metadata about the running application. Can also be used as" @@ -31,11 +32,6 @@ summary="Application metadata", ) async def get_index() -> Metadata: - """GET ``/`` (the app's internal root). - - By convention, this endpoint returns only the application's metadata. - """ return get_metadata( - package_name="vo-cutouts", - application_name=config.name, + package_name="vo-cutouts", application_name=config.name ) diff --git a/src/vocutouts/main.py b/src/vocutouts/main.py index 2a71891..e5b1cb5 100644 --- a/src/vocutouts/main.py +++ b/src/vocutouts/main.py @@ -17,15 +17,16 @@ from safir.logging import Profile, configure_logging, configure_uvicorn_logging from safir.middleware.ivoa import CaseInsensitiveQueryMiddleware from safir.middleware.x_forwarded import XForwardedMiddleware +from safir.models import ErrorModel +from safir.slack.webhook import SlackRouteErrorHandler from .config import config -from .handlers.external import external_router -from .handlers.internal import internal_router +from .handlers import external, internal from .policy import ImageCutoutPolicy from .uws.dependencies import uws_dependency from .uws.errors import install_error_handlers -__all__ = ["app", "lifespan"] +__all__ = ["app"] @asynccontextmanager @@ -63,11 +64,14 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: """The main FastAPI application for vo-cutouts.""" # Attach the routers. -app.include_router(internal_router) +app.include_router(internal.router) app.include_router( - external_router, + external.router, prefix=config.path_prefix, - responses={401: {"description": "Unauthenticated"}}, + responses={ + 401: {"description": "Unauthenticated"}, + 403: {"description": "Permission denied", "model": ErrorModel}, + }, ) # Install middleware. @@ -76,3 +80,11 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: # Install error handlers. install_error_handlers(app) + +# Configure Slack alerts. +if config.slack_webhook: + logger = structlog.get_logger("vocutouts") + SlackRouteErrorHandler.initialize( + config.slack_webhook.get_secret_value(), "vo-cutouts", logger + ) + logger.debug("Initialized Slack webhook") diff --git a/src/vocutouts/uws/config.py b/src/vocutouts/uws/config.py index 2eb341d..a363e7f 100644 --- a/src/vocutouts/uws/config.py +++ b/src/vocutouts/uws/config.py @@ -24,6 +24,12 @@ class encapsulates the configuration of the UWS component that may vary by `vocutouts.uws.dependencies.UWSDependency` object. """ + arq_mode: ArqMode + """What mode to use for the arq queue.""" + + arq_redis_settings: RedisSettings + """Settings for Redis for the arq queue.""" + execution_duration: timedelta """Maximum execution time in seconds. @@ -42,12 +48,6 @@ class encapsulates the configuration of the UWS component that may vary by database_url: str """URL for the metadata database.""" - arq_mode: ArqMode - """What mode to use for the arq queue.""" - - arq_redis_settings: RedisSettings - """Settings for Redis for the arq queue.""" - signing_service_account: str """Email of service account to use for signed URLs. @@ -59,6 +59,9 @@ class encapsulates the configuration of the UWS component that may vary by database_password: SecretStr | None = None """Password for the database.""" + slack_webhook: SecretStr | None = None + """Slack incoming webhook for reporting errors.""" + url_lifetime: timedelta = timedelta(minutes=15) """How long result URLs should be valid for.""" diff --git a/src/vocutouts/uws/exceptions.py b/src/vocutouts/uws/exceptions.py index 07f6dc9..27b176c 100644 --- a/src/vocutouts/uws/exceptions.py +++ b/src/vocutouts/uws/exceptions.py @@ -6,6 +6,8 @@ from __future__ import annotations +from safir.slack.webhook import SlackIgnoredException + from .models import ErrorCode, ErrorType, UWSJobError __all__ = [ @@ -62,7 +64,7 @@ def __str__(self) -> str: return self.message -class MultiValuedParameterError(UWSError): +class MultiValuedParameterError(UWSError, SlackIgnoredException): """Multiple values not allowed for this parameter.""" def __init__(self, message: str) -> None: @@ -70,7 +72,7 @@ def __init__(self, message: str) -> None: self.status_code = 422 -class PermissionDeniedError(UWSError): +class PermissionDeniedError(UWSError, SlackIgnoredException): """User does not have access to this resource.""" def __init__(self, message: str) -> None: @@ -150,7 +152,7 @@ def __init__( super().__init__(ErrorType.TRANSIENT, error_code, message, detail) -class UsageError(UWSError): +class UsageError(UWSError, SlackIgnoredException): """Invalid parameters were passed to a UWS API.""" def __init__(self, message: str, detail: str | None = None) -> None: @@ -158,7 +160,7 @@ def __init__(self, message: str, detail: str | None = None) -> None: self.status_code = 422 -class DataMissingError(UWSError): +class DataMissingError(UWSError, SlackIgnoredException): """The data requested does not exist for that job.""" def __init__(self, message: str) -> None: diff --git a/src/vocutouts/uws/handlers.py b/src/vocutouts/uws/handlers.py index 1f9f609..9305016 100644 --- a/src/vocutouts/uws/handlers.py +++ b/src/vocutouts/uws/handlers.py @@ -13,7 +13,7 @@ .. code-block:: python - external_router.include_router(uws_router, prefix="/jobs") + router.include_router(uws_router, prefix="/jobs") """ from datetime import datetime @@ -27,6 +27,7 @@ auth_dependency, auth_logger_dependency, ) +from safir.slack.webhook import SlackRouteErrorHandler from structlog.stdlib import BoundLogger from .dependencies import ( @@ -37,11 +38,11 @@ from .exceptions import DataMissingError, ParameterError, PermissionDeniedError from .models import ExecutionPhase, UWSJobParameter -__all__ = ["uws_router"] - -uws_router = APIRouter() +uws_router = APIRouter(route_class=SlackRouteErrorHandler) """FastAPI router for all external handlers.""" +__all__ = ["uws_router"] + @uws_router.get( "", diff --git a/tests/conftest.py b/tests/conftest.py index 6eb2671..dc60cc9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,6 +7,7 @@ import pytest import pytest_asyncio +import respx import structlog from asgi_lifespan import LifespanManager from fastapi import FastAPI @@ -15,6 +16,7 @@ from safir.database import create_database_engine, initialize_database from safir.dependencies.db_session import db_session_dependency from safir.testing.gcs import MockStorageClient, patch_google_storage +from safir.testing.slack import MockSlackWebhook, mock_slack_webhook from vocutouts import main from vocutouts.config import config @@ -69,6 +71,13 @@ def mock_google_storage() -> Iterator[MockStorageClient]: ) +@pytest.fixture(autouse=True) +def mock_slack(respx_mock: respx.Router) -> MockSlackWebhook: + assert config.slack_webhook + webhook = config.slack_webhook.get_secret_value() + return mock_slack_webhook(webhook, respx_mock) + + @pytest.fixture def runner(uws_factory: UWSFactory, arq_queue: MockArqQueue) -> MockJobRunner: return MockJobRunner(uws_factory, arq_queue) diff --git a/tests/handlers/async_test.py b/tests/handlers/async_test.py index 3fadbda..303a47d 100644 --- a/tests/handlers/async_test.py +++ b/tests/handlers/async_test.py @@ -8,6 +8,7 @@ import pytest from fastapi import FastAPI from httpx import ASGITransport, AsyncClient +from safir.testing.slack import MockSlackWebhook from vocutouts.uws.models import UWSJobResult @@ -155,7 +156,9 @@ async def test_redirect(app: FastAPI) -> None: @pytest.mark.asyncio -async def test_bad_parameters(client: AsyncClient) -> None: +async def test_bad_parameters( + client: AsyncClient, mock_slack: MockSlackWebhook +) -> None: bad_params: list[dict[str, str]] = [ {}, {"pos": "RANGE 0 360 -2 2"}, @@ -179,3 +182,6 @@ async def test_bad_parameters(client: AsyncClient) -> None: ) assert r.status_code == 422, f"Parameters {params}" assert r.text.startswith("UsageError") + + # None of these requests should have been reported to Slack. + assert mock_slack.messages == [] diff --git a/tests/handlers/error_test.py b/tests/handlers/error_test.py index 9742421..3c38891 100644 --- a/tests/handlers/error_test.py +++ b/tests/handlers/error_test.py @@ -2,9 +2,12 @@ from __future__ import annotations +from unittest.mock import ANY + import pytest from httpx import AsyncClient from safir.database import create_database_engine +from safir.testing.slack import MockSlackWebhook from sqlalchemy.exc import ProgrammingError from vocutouts.config import config @@ -12,11 +15,16 @@ @pytest.mark.asyncio -async def test_uncaught_error(client: AsyncClient) -> None: - # Drop the schema, which will ensure that all database errors will throw a - # SQLAlchemy exception. Previously this would result in a 500 error with - # no meaningful information and no exception traceback due a bug in - # swallowing errors in subapps. +async def test_uncaught_error( + client: AsyncClient, mock_slack: MockSlackWebhook +) -> None: + """Check that uncaught errors are reported to Slack. + + Drop the schema, which will ensure that all database errors will throw a + SQLAlchemy exception. Previously this would result in a 500 error with no + meaningful information and no exception traceback due a bug in swallowing + errors in subapps. + """ engine = create_database_engine( config.database_url, config.database_password ) @@ -30,3 +38,45 @@ async def test_uncaught_error(client: AsyncClient) -> None: headers={"X-Auth-Request-User": "someone"}, params={"ID": "1:2:band:id", "Pos": "CIRCLE 0 -2 2"}, ) + + # Ensure that this message is reported to Slack. + assert mock_slack.messages == [ + { + "blocks": [ + { + "text": { + "text": "Uncaught exception in vo-cutouts", + "type": "mrkdwn", + "verbatim": True, + }, + "type": "section", + }, + { + "fields": [ + { + "text": "*Exception type*\nProgrammingError", + "type": "mrkdwn", + "verbatim": True, + }, + { + "text": ANY, + "type": "mrkdwn", + "verbatim": True, + }, + ], + "type": "section", + }, + { + "text": { + "text": ANY, + "type": "mrkdwn", + "verbatim": True, + }, + "type": "section", + }, + { + "type": "divider", + }, + ], + }, + ] diff --git a/tests/handlers/sync_test.py b/tests/handlers/sync_test.py index de26219..08c5a3a 100644 --- a/tests/handlers/sync_test.py +++ b/tests/handlers/sync_test.py @@ -6,6 +6,7 @@ import pytest from httpx import AsyncClient +from safir.testing.slack import MockSlackWebhook from vocutouts.uws.models import UWSJobResult @@ -51,7 +52,9 @@ async def run_job(job_id: str) -> None: @pytest.mark.asyncio -async def test_bad_parameters(client: AsyncClient) -> None: +async def test_bad_parameters( + client: AsyncClient, mock_slack: MockSlackWebhook +) -> None: bad_params: list[dict[str, str]] = [ {}, {"pos": "RANGE 0 360 -2 2"}, @@ -81,3 +84,6 @@ async def test_bad_parameters(client: AsyncClient) -> None: ) assert r.status_code == 422, f"Parameters {params}" assert r.text.startswith("UsageError") + + # None of these requests should have been reported to Slack. + assert mock_slack.messages == [] diff --git a/tests/support/uws.py b/tests/support/uws.py index 3cb3888..483780b 100644 --- a/tests/support/uws.py +++ b/tests/support/uws.py @@ -68,16 +68,17 @@ def build_uws_config() -> UWSConfig: db_name = os.environ["POSTGRES_DB"] database_url = f"postgresql://{db_user}@{db_host}:{db_port}/{db_name}" return UWSConfig( - execution_duration=timedelta(minutes=10), - lifetime=timedelta(days=1), - database_url=database_url, - database_password=SecretStr(os.environ["POSTGRES_PASSWORD"]), arq_mode=ArqMode.test, arq_redis_settings=RedisSettings( host=os.environ["REDIS_HOST"], port=int(os.environ["REDIS_6379_TCP_PORT"]), ), + execution_duration=timedelta(minutes=10), + lifetime=timedelta(days=1), + database_url=database_url, + database_password=SecretStr(os.environ["POSTGRES_PASSWORD"]), signing_service_account="", + slack_webhook=SecretStr("https://example.com/fake-webhook"), ) diff --git a/tests/uws/conftest.py b/tests/uws/conftest.py index ccc5739..178db6b 100644 --- a/tests/uws/conftest.py +++ b/tests/uws/conftest.py @@ -8,6 +8,7 @@ import pytest import pytest_asyncio +import respx import structlog from asgi_lifespan import LifespanManager from fastapi import FastAPI @@ -18,6 +19,7 @@ from safir.middleware.ivoa import CaseInsensitiveQueryMiddleware from safir.middleware.x_forwarded import XForwardedMiddleware from safir.testing.gcs import MockStorageClient, patch_google_storage +from safir.testing.slack import MockSlackWebhook, mock_slack_webhook from sqlalchemy.ext.asyncio import async_scoped_session from structlog.stdlib import BoundLogger @@ -94,6 +96,16 @@ def mock_google_storage() -> Iterator[MockStorageClient]: ) +@pytest.fixture(autouse=True) +def mock_slack( + uws_config: UWSConfig, respx_mock: respx.Router +) -> MockSlackWebhook: + assert uws_config.slack_webhook + return mock_slack_webhook( + uws_config.slack_webhook.get_secret_value(), respx_mock + ) + + @pytest.fixture def runner(uws_factory: UWSFactory, arq_queue: MockArqQueue) -> MockJobRunner: return MockJobRunner(uws_factory, arq_queue) diff --git a/tests/uws/errors_test.py b/tests/uws/errors_test.py index e354afe..c07eb9d 100644 --- a/tests/uws/errors_test.py +++ b/tests/uws/errors_test.py @@ -6,6 +6,7 @@ import pytest from httpx import AsyncClient +from safir.testing.slack import MockSlackWebhook from vocutouts.uws.dependencies import UWSFactory from vocutouts.uws.models import UWSJobParameter @@ -20,7 +21,9 @@ class PostTest: @pytest.mark.asyncio -async def test_errors(client: AsyncClient, uws_factory: UWSFactory) -> None: +async def test_errors( + client: AsyncClient, uws_factory: UWSFactory, mock_slack: MockSlackWebhook +) -> None: job_service = uws_factory.create_job_service() await job_service.create( "user", @@ -131,3 +134,6 @@ async def test_errors(client: AsyncClient, uws_factory: UWSFactory) -> None: ) assert r.status_code == 422 assert r.text.startswith("UsageError") + + # None of these errors should have produced Slack errors. + assert mock_slack.messages == [] diff --git a/tests/uws/job_error_test.py b/tests/uws/job_error_test.py index 13a40e6..cd9f219 100644 --- a/tests/uws/job_error_test.py +++ b/tests/uws/job_error_test.py @@ -5,6 +5,7 @@ import pytest from httpx import AsyncClient from safir.datetime import isodatetime +from safir.testing.slack import MockSlackWebhook from vocutouts.uws.dependencies import UWSFactory from vocutouts.uws.exceptions import TaskFatalError, TaskTransientError @@ -49,7 +50,10 @@ @pytest.mark.asyncio async def test_temporary_error( - client: AsyncClient, runner: MockJobRunner, uws_factory: UWSFactory + client: AsyncClient, + runner: MockJobRunner, + uws_factory: UWSFactory, + mock_slack: MockSlackWebhook, ) -> None: job_service = uws_factory.create_job_service() await job_service.create( @@ -97,10 +101,16 @@ async def test_temporary_error( "UsageError Something failed" ) + # For now, this shouldn't have resulted in Slack errors. + assert mock_slack.messages == [] + @pytest.mark.asyncio async def test_fatal_error( - client: AsyncClient, runner: MockJobRunner, uws_factory: UWSFactory + client: AsyncClient, + runner: MockJobRunner, + uws_factory: UWSFactory, + mock_slack: MockSlackWebhook, ) -> None: job_service = uws_factory.create_job_service() await job_service.create( @@ -142,10 +152,16 @@ async def test_fatal_error( "Error Whoops\n\nSome details" ) + # For now, this shouldn't have resulted in Slack errors. + assert mock_slack.messages == [] + @pytest.mark.asyncio async def test_unknown_error( - client: AsyncClient, runner: MockJobRunner, uws_factory: UWSFactory + client: AsyncClient, + runner: MockJobRunner, + uws_factory: UWSFactory, + mock_slack: MockSlackWebhook, ) -> None: job_service = uws_factory.create_job_service() await job_service.create( @@ -187,3 +203,6 @@ async def test_unknown_error( "Error Unknown error executing task\n\n" "ValueError: Unknown exception" ) + + # For now, this shouldn't have resulted in Slack errors. + assert mock_slack.messages == [] diff --git a/tox.ini b/tox.ini index 68cf3a4..7ef66d1 100644 --- a/tox.ini +++ b/tox.ini @@ -53,8 +53,9 @@ setenv = CUTOUT_DATABASE_URL = postgresql://vo-cutouts@localhost/vo-cutouts CUTOUT_DATABASE_PASSWORD = INSECURE-PASSWORD CUTOUT_ARQ_QUEUE_URL = redis://localhost/0 - CUTOUT_SERVICE_ACCOUNT = "vo-cutouts@example.com" - CUTOUT_STORAGE_URL = "s3://some-bucket" + CUTOUT_SERVICE_ACCOUNT = vo-cutouts@example.com + CUTOUT_SLACK_WEBHOOK = https://example.com/fake-webhook + CUTOUT_STORAGE_URL = s3://some-bucket POSTGRES_USER = vo-cutouts POSTGRES_DB = vo-cutouts POSTGRES_PASSWORD = INSECURE-PASSWORD