Skip to content

Commit

Permalink
♻️ improve error handling of pricing plans in webserver (#4980)
Browse files Browse the repository at this point in the history
  • Loading branch information
matusdrobuliak66 authored Nov 10, 2023
1 parent 5e3e596 commit d77e335
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .env-devel
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ TRAEFIK_SIMCORE_ZONE=internal_simcore_stack

# NOTE: WEBSERVER_SESSION_SECRET_KEY = $(python3 -c "from cryptography.fernet import Fernet; print(Fernet.generate_key())")

WEBSERVER_CREDIT_COMPUTATION_ENABLED=0
WEBSERVER_CREDIT_COMPUTATION_ENABLED=1

WEBSERVER_LOGLEVEL=INFO

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ async def on_payment_completed(
credit_transaction_id = await rut_api.create_credit_transaction(
product_name=transaction.product_name,
wallet_id=transaction.wallet_id,
wallet_name="id={transaction.wallet_id}",
wallet_name=f"id={transaction.wallet_id}",
user_id=transaction.user_id,
user_email=transaction.user_email,
osparc_credits=transaction.osparc_credits,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ def http404_error_handler(
request: Request, # pylint: disable=unused-argument
error: CustomResourceUsageTrackerError,
) -> JSONResponse:
return JSONResponse(status_code=404, content={"message": error.msg_template})
return JSONResponse(status_code=404, content={"message": f"{error.msg_template}"})
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ async def get_pricing_unit(
row = result.first()
if row is None:
raise CustomResourceUsageTrackerError(
msg=f"Pricing unit id {pricing_unit_id} not found"
msg=f"Pricing plan {pricing_plan_id} and pricing unit {pricing_unit_id} for product {product_name} not found"
)
return PricingUnitsDB.from_orm(row)

Expand Down Expand Up @@ -723,6 +723,6 @@ async def get_pricing_unit_cost_by_id(
row = result.first()
if row is None:
raise CustomResourceUsageTrackerError(
msg=f"Pricing unit cosd id {pricing_unit_cost_id} not found in the resource_tracker_pricing_unit_costs table",
msg=f"Pricing unit cost id {pricing_unit_cost_id} not found in the resource_tracker_pricing_unit_costs table",
)
return PricingUnitCostsDB.from_orm(row)
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
MSG_CATALOG_SERVICE_UNAVAILABLE: Final[
str
] = "Currently catalog service is unavailable, please try again later"

MSG_CATALOG_SERVICE_NOT_FOUND: Final[str] = "Not Found"
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from yarl import URL

from .._meta import api_version_prefix
from ._constants import MSG_CATALOG_SERVICE_UNAVAILABLE
from ._constants import MSG_CATALOG_SERVICE_NOT_FOUND, MSG_CATALOG_SERVICE_UNAVAILABLE
from .settings import get_plugin_settings

_logger = logging.getLogger(__name__)
Expand All @@ -39,7 +39,14 @@ def _handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]:

yield session

except (asyncio.TimeoutError, ClientConnectionError, ClientResponseError) as err:
except ClientResponseError as err:
if err.status == 404:
raise web.HTTPNotFound(reason=MSG_CATALOG_SERVICE_NOT_FOUND)
raise web.HTTPServiceUnavailable(
reason=MSG_CATALOG_SERVICE_UNAVAILABLE
) from err

except (asyncio.TimeoutError, ClientConnectionError) as err:
_logger.debug("Request to catalog service failed: %s", err)
raise web.HTTPServiceUnavailable(
reason=MSG_CATALOG_SERVICE_UNAVAILABLE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ async def get_default_service_pricing_plan(
)
with handle_client_exceptions(app) as session:
async with session.get(url) as response:
response.raise_for_status()
body: dict = await response.json()
return parse_obj_as(ServicePricingPlanGet, body)

Expand All @@ -119,6 +120,7 @@ async def get_pricing_plan_unit(
)
with handle_client_exceptions(app) as session:
async with session.get(url) as response:
response.raise_for_status()
body: dict = await response.json()
return parse_obj_as(PricingUnitGet, body)

Expand All @@ -139,6 +141,7 @@ async def sum_total_available_credits_in_the_wallet(
)
with handle_client_exceptions(app) as session:
async with session.post(url) as response:
response.raise_for_status()
body: dict = await response.json()
return WalletTotalCredits.construct(**body)

Expand Down Expand Up @@ -168,6 +171,7 @@ async def add_credits_to_wallet(
}
with handle_client_exceptions(app) as session:
async with session.post(url, json=body) as response:
response.raise_for_status()
output: dict = await response.json()
return output

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE: Final[
str
] = "Currently resource usage tracker service is unavailable, please try again later"

MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND: Final[str] = "Not Found"
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
from aiohttp.client_exceptions import ClientConnectionError, ClientResponseError
from servicelib.aiohttp.client_session import get_client_session

from ._constants import MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE
from ._constants import (
MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND,
MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE,
)

_logger = logging.getLogger(__name__)

Expand All @@ -18,8 +21,14 @@ def handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]:
session: ClientSession = get_client_session(app)

yield session
except (ClientResponseError) as err:
if err.status == 404:
raise web.HTTPNotFound(reason=MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND)
raise web.HTTPServiceUnavailable(
reason=MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE
) from err

except (asyncio.TimeoutError, ClientConnectionError, ClientResponseError) as err:
except (asyncio.TimeoutError, ClientConnectionError) as err:
_logger.debug("Request to resource usage tracker service failed: %s", err)
raise web.HTTPServiceUnavailable(
reason=MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# pylint:disable=unused-variable
# pylint:disable=unused-argument
# pylint:disable=redefined-outer-name

import re
import urllib.parse

import pytest
from aiohttp import web
from aiohttp.test_utils import TestClient
from models_library.api_schemas_resource_usage_tracker.pricing_plans import (
ServicePricingPlanGet,
)
from models_library.utils.fastapi_encoders import jsonable_encoder
from pydantic import parse_obj_as
from pytest_simcore.aioresponses_mocker import AioResponsesMock
from pytest_simcore.helpers.utils_assert import assert_status
from pytest_simcore.helpers.utils_login import UserInfoDict
from settings_library.resource_usage_tracker import ResourceUsageTrackerSettings
from simcore_service_webserver.db.models import UserRole
from simcore_service_webserver.resource_usage.settings import get_plugin_settings


@pytest.fixture
def mock_rut_api_responses(
client: TestClient, aioresponses_mocker: AioResponsesMock
) -> AioResponsesMock:
assert client.app
settings: ResourceUsageTrackerSettings = get_plugin_settings(client.app)

service_pricing_plan_get = parse_obj_as(
ServicePricingPlanGet,
ServicePricingPlanGet.Config.schema_extra["examples"][0],
)
aioresponses_mocker.get(
re.compile(f"^{settings.api_base_url}/services/+.+$"),
payload=jsonable_encoder(service_pricing_plan_get),
)

return aioresponses_mocker


@pytest.mark.parametrize(
"user_role,expected",
[
(UserRole.ANONYMOUS, web.HTTPUnauthorized),
(UserRole.GUEST, web.HTTPOk),
(UserRole.USER, web.HTTPOk),
(UserRole.TESTER, web.HTTPOk),
],
)
async def test_get_service_pricinp_plan_role_access_rights(
client: TestClient,
logged_user: UserInfoDict,
mock_rut_api_responses: AioResponsesMock,
expected: type[web.HTTPException],
):
assert client.app
assert client.app.router
url = client.app.router["get_service_pricing_plan"].url_for(
service_key=urllib.parse.quote("simcore/services/dynamic/someservice", safe=""),
service_version="3.4.5",
)
response = await client.get(f"{url}")
await assert_status(response, expected)


@pytest.fixture
def mock_catalog_get_service_pricing_plan_not_found(
client: TestClient, aioresponses_mocker: AioResponsesMock
) -> AioResponsesMock:
assert client.app
settings: ResourceUsageTrackerSettings = get_plugin_settings(client.app)
url_pattern = re.compile(f"^{settings.base_url}+/.*$")

aioresponses_mocker.get(url_pattern, exception=web.HTTPNotFound)
return aioresponses_mocker


@pytest.mark.parametrize(
"user_role,expected",
[
(UserRole.TESTER, web.HTTPNotFound),
],
)
async def test_get_service_pricing_plan_raises_not_found_error(
client: TestClient,
logged_user: UserInfoDict,
mock_catalog_get_service_pricing_plan_not_found: AioResponsesMock,
expected: type[web.HTTPException],
):
assert client.app
assert client.app.router
url = client.app.router["get_service_pricing_plan"].url_for(
service_key="simcore%2Fservices%2Fdynamic%2Fsomeservice",
service_version="3.4.5",
)
response = await client.get(f"{url}")
await assert_status(response, expected)

0 comments on commit d77e335

Please sign in to comment.