Skip to content

Commit

Permalink
🐛 Improved Error Handling for Missing Billing Details (#6418)
Browse files Browse the repository at this point in the history
  • Loading branch information
pcrespov authored Sep 21, 2024
1 parent 2990728 commit e880d71
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 5 deletions.
2 changes: 1 addition & 1 deletion packages/models-library/src/models_library/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class UserBillingDetails(BaseModel):
address: str | None
city: str | None
state: str | None = Field(description="State, province, canton, ...")
country: str
country: str # Required for taxes
postal_code: str | None
phone: str | None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ async def init_creation_of_wallet_payment(
Raises:
UserNotFoundError
WalletAccessForbiddenError
BillingDetailsNotFoundError
"""

# wallet: check permissions
Expand All @@ -293,6 +294,7 @@ async def init_creation_of_wallet_payment(
# user info
user = await get_user_display_and_id_names(app, user_id=user_id)
user_invoice_address = await get_user_invoice_address(app, user_id=user_id)

# stripe info
product_stripe_info = await get_product_stripe_info(app, product_name=product_name)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from ..db.models import user_to_groups
from ..db.plugin import get_database_engine
from .exceptions import BillingDetailsNotFoundError
from .schemas import Permission

_ALL = None
Expand Down Expand Up @@ -203,9 +204,12 @@ async def new_user_details(
async def get_user_billing_details(
engine: Engine, user_id: UserID
) -> UserBillingDetails:
"""
Raises:
BillingDetailsNotFoundError
"""
async with engine.acquire() as conn:
user_billing_details = await UsersRepo.get_billing_details(conn, user_id)
if not user_billing_details:
msg = f"Missing biling details for user {user_id}"
raise ValueError(msg)
raise BillingDetailsNotFoundError(user_id=user_id)
return UserBillingDetails.from_orm(user_billing_details)
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,8 @@ class AlreadyPreRegisteredError(UsersBaseError):
msg_template = (
"Found {num_found} matches for '{email}'. Cannot pre-register existing user"
)


class BillingDetailsNotFoundError(UsersBaseError):
# NOTE: this is for internal log and should not be transmitted to the final user
msg_template = "Billing details are missing for user_id={user_id}. TIP: Check whether this user is pre-registered"
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@
MSG_PRICE_NOT_DEFINED_ERROR: Final[
str
] = "No payments are accepted until this product has a price"

MSG_BILLING_DETAILS_NOT_DEFINED_ERROR: Final[str] = (
"Payments cannot be processed: Required billing details (e.g. country for tax) are missing from your account."
"Please contact support to resolve this configuration issue."
)
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
parse_request_path_parameters_as,
)
from servicelib.aiohttp.typing_extension import Handler
from servicelib.error_codes import create_error_code
from servicelib.logging_utils import LogExtra, get_log_record_extra
from servicelib.request_keys import RQT_USERID_KEY

from .._constants import RQ_PRODUCT_KEY
Expand All @@ -36,10 +38,16 @@
)
from ..products.errors import BelowMinimumPaymentError, ProductPriceNotDefinedError
from ..security.decorators import permission_required
from ..users.exceptions import UserDefaultWalletNotFoundError
from ..users.exceptions import (
BillingDetailsNotFoundError,
UserDefaultWalletNotFoundError,
)
from ..utils_aiohttp import envelope_json_response
from . import _api
from ._constants import MSG_PRICE_NOT_DEFINED_ERROR
from ._constants import (
MSG_BILLING_DETAILS_NOT_DEFINED_ERROR,
MSG_PRICE_NOT_DEFINED_ERROR,
)
from .errors import WalletAccessForbiddenError, WalletNotFoundError

_logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -80,6 +88,20 @@ async def wrapper(request: web.Request) -> web.StreamResponse:
except ProductPriceNotDefinedError as exc:
raise web.HTTPConflict(reason=MSG_PRICE_NOT_DEFINED_ERROR) from exc

except BillingDetailsNotFoundError as exc:
error_code = create_error_code(exc)
log_extra: LogExtra = {}
if user_id := getattr(exc, "user_id", None):
log_extra = get_log_record_extra(user_id=user_id) or {}

log_msg = f"{exc} [{error_code}]"
_logger.exception(
log_msg,
extra={"error_code": error_code, **log_extra},
)
user_msg = f"{MSG_BILLING_DETAILS_NOT_DEFINED_ERROR} ({error_code})"
raise web.HTTPServiceUnavailable(reason=user_msg) from exc

return wrapper


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
PaymentsSettings,
get_plugin_settings,
)
from simcore_service_webserver.wallets._constants import (
MSG_BILLING_DETAILS_NOT_DEFINED_ERROR,
)

OpenApiDict: TypeAlias = dict[str, Any]

Expand Down Expand Up @@ -312,6 +315,29 @@ async def test_complete_payment_errors(
send_message.assert_called_once()


async def test_billing_info_missing_error(
latest_osparc_price: Decimal,
client: TestClient,
logged_user_wallet: WalletGet,
):
# NOTE: setup_user_pre_registration_details_db is not setup to emulate missing pre-registration

assert client.app
wallet = logged_user_wallet

# Pay
response = await client.post(
f"/v0/wallets/{wallet.wallet_id}/payments", json={"priceDollars": 25}
)
data, error = await assert_status(response, status.HTTP_503_SERVICE_UNAVAILABLE)

assert not data
assert MSG_BILLING_DETAILS_NOT_DEFINED_ERROR in error["message"]

assert response.reason
assert MSG_BILLING_DETAILS_NOT_DEFINED_ERROR in response.reason


async def test_payment_not_found(
latest_osparc_price: Decimal,
client: TestClient,
Expand Down

0 comments on commit e880d71

Please sign in to comment.