diff --git a/server/polar/subscription/email_templates/benefits/custom/precondition_failed.html b/server/polar/subscription/email_templates/benefits/custom/precondition_failed.html new file mode 100644 index 0000000000..1a9f8ef1dd --- /dev/null +++ b/server/polar/subscription/email_templates/benefits/custom/precondition_failed.html @@ -0,0 +1,6 @@ +{% extends "base.html" %} + +{% block body %} +
We had trouble granting you access to the benefit {{ subscription_benefit.description }}.
+{% endblock %} \ No newline at end of file diff --git a/server/polar/subscription/service/benefits/__init__.py b/server/polar/subscription/service/benefits/__init__.py index 29d550f45c..6fd167d959 100644 --- a/server/polar/subscription/service/benefits/__init__.py +++ b/server/polar/subscription/service/benefits/__init__.py @@ -7,8 +7,8 @@ from ...schemas import SubscriptionBenefitUpdate from .base import ( SB, - SubscriptionBenefitGrantError, - SubscriptionBenefitRevokeError, + SubscriptionBenefitPreconditionError, + SubscriptionBenefitRetriableError, SubscriptionBenefitServiceError, SubscriptionBenefitServiceProtocol, ) @@ -30,8 +30,8 @@ def get_subscription_benefit_service( __all__ = [ "SubscriptionBenefitServiceProtocol", - "SubscriptionBenefitGrantError", - "SubscriptionBenefitRevokeError", + "SubscriptionBenefitPreconditionError", + "SubscriptionBenefitRetriableError", "SubscriptionBenefitServiceError", "get_subscription_benefit_service", ] diff --git a/server/polar/subscription/service/benefits/base.py b/server/polar/subscription/service/benefits/base.py index e17702df63..017e9a41ed 100644 --- a/server/polar/subscription/service/benefits/base.py +++ b/server/polar/subscription/service/benefits/base.py @@ -1,7 +1,7 @@ -from typing import Protocol, TypeVar +from typing import Any, Protocol, TypeVar from polar.exceptions import PolarError -from polar.models import SubscriptionBenefit +from polar.models import Subscription, SubscriptionBenefit from polar.postgres import AsyncSession from ...schemas import SubscriptionBenefitUpdate @@ -11,12 +11,60 @@ class SubscriptionBenefitServiceError(PolarError): ... -class SubscriptionBenefitGrantError(SubscriptionBenefitServiceError): - ... - - -class SubscriptionBenefitRevokeError(SubscriptionBenefitServiceError): - ... +class SubscriptionBenefitRetriableError(SubscriptionBenefitServiceError): + """ + A retriable error occured while granting or revoking the benefit. + """ + + defer_seconds: int + "Number of seconds to wait before retrying." + + def __init__(self, defer_seconds: int) -> None: + self.defer_seconds = defer_seconds + message = f"An error occured. We'll retry in {defer_seconds} seconds." + super().__init__(message) + + +class SubscriptionBenefitPreconditionError(SubscriptionBenefitServiceError): + """ + Some conditions are missing to grant the benefit. + + It accepts an email subject and body templates. When set, an email will + be sent to the backer to explain them what happened. It'll be generated with + the following context: + + ```py + class Context: + subscription: Subscription + subscription_tier: SubscriptionTier + subscription_benefit: SubscriptionBenefit + user: User + ``` + + An additional context dictionary can also be passed. + """ + + def __init__( + self, + message: str, + *, + email_subject: str | None = None, + email_body_template: str | None = None, + email_extra_context: dict[str, Any] | None = None, + ) -> None: + """ + Args: + message: The plain error message + email_subject: Template string for the email subject + we'll send to the backer. + email_body_template: Path to the email template body + we'll send to the backer. + It's expected to be under `subscription/email_templates` directory. + """ + self.email_subject = email_subject + self.email_body_template = email_body_template + self.email_extra_context = email_extra_context or {} + super().__init__(message) SB = TypeVar("SB", bound=SubscriptionBenefit, contravariant=True) @@ -24,16 +72,67 @@ class SubscriptionBenefitRevokeError(SubscriptionBenefitServiceError): class SubscriptionBenefitServiceProtocol(Protocol[SB, SBU]): + """ + Protocol that should be implemented by each benefit type service. + + It allows to implement very customizable and specific logic to fulfill the benefit. + """ + session: AsyncSession def __init__(self, session: AsyncSession) -> None: self.session = session - async def grant(self, benefit: SB) -> None: + async def grant( + self, benefit: SB, subscription: Subscription, *, attempt: int = 1 + ) -> None: + """ + Executes the logic to grant a benefit to a backer. + + Args: + benefit: The SubscriptionBenefit to grant. + subscription: The Subscription we should grant this benefit to. + Use it to access the underlying backer user. + attempt: Number of times we attempted to grant the benefit. + Useful for the worker to implement retry logic. + + Raises: + SubscriptionBenefitRetriableError: An temporary error occured, + we should be able to retry later. + SubscriptionBenefitPreconditionError: Some conditions are missing + to grant the benefit. + """ ... - async def revoke(self, benefit: SB) -> None: + async def revoke( + self, benefit: SB, subscription: Subscription, *, attempt: int = 1 + ) -> None: + """ + Executes the logic to revoke a benefit from a backer. + + Args: + benefit: The SubscriptionBenefit to revoke. + subscription: The Subscription we should revoke this benefit from. + Use it to access the underlying backer user. + attempt: Number of times we attempted to revoke the benefit. + Useful for the worker to implement retry logic. + + Raises: + SubscriptionBenefitRetriableError: An temporary error occured, + we should be able to retry later. + """ ... async def requires_update(self, benefit: SB, update: SBU) -> bool: + """ + Determines if a benefit update requires to trigger the granting logic again. + + This method is called whenever a benefit is updated. If it returns `True`, the + granting logic will be re-executed again for all the backers. + + Args: + benefit: The updated SubscriptionBenefit. + update: The SubscriptionBenefitUpdate schema. + Use it to check which fields have been updated. + """ ... diff --git a/server/polar/subscription/service/benefits/custom.py b/server/polar/subscription/service/benefits/custom.py index 291244de87..dfbc2053fd 100644 --- a/server/polar/subscription/service/benefits/custom.py +++ b/server/polar/subscription/service/benefits/custom.py @@ -1,3 +1,4 @@ +from polar.models import Subscription from polar.models.subscription_benefit import SubscriptionBenefitCustom from ...schemas import SubscriptionBenefitCustomUpdate @@ -9,10 +10,22 @@ class SubscriptionBenefitCustomService( SubscriptionBenefitCustom, SubscriptionBenefitCustomUpdate ] ): - async def grant(self, benefit: SubscriptionBenefitCustom) -> None: + async def grant( + self, + benefit: SubscriptionBenefitCustom, + subscription: Subscription, + *, + attempt: int = 1, + ) -> None: return - async def revoke(self, benefit: SubscriptionBenefitCustom) -> None: + async def revoke( + self, + benefit: SubscriptionBenefitCustom, + subscription: Subscription, + *, + attempt: int = 1, + ) -> None: return async def requires_update( diff --git a/server/polar/subscription/service/subscription_benefit_grant.py b/server/polar/subscription/service/subscription_benefit_grant.py index 0ed418ae88..f0d57b16d1 100644 --- a/server/polar/subscription/service/subscription_benefit_grant.py +++ b/server/polar/subscription/service/subscription_benefit_grant.py @@ -1,14 +1,23 @@ from collections.abc import Sequence +import structlog from sqlalchemy import select +from polar.email.renderer import get_email_renderer +from polar.email.sender import get_email_sender from polar.kit.services import ResourceServiceReader +from polar.logging import Logger from polar.models import Subscription, SubscriptionBenefit, SubscriptionBenefitGrant from polar.postgres import AsyncSession from polar.worker import enqueue_job from ..schemas import SubscriptionBenefitUpdate -from .benefits import get_subscription_benefit_service +from .benefits import ( + SubscriptionBenefitPreconditionError, + get_subscription_benefit_service, +) + +log: Logger = structlog.get_logger() class SubscriptionBenefitGrantService(ResourceServiceReader[SubscriptionBenefitGrant]): @@ -17,6 +26,8 @@ async def grant_benefit( session: AsyncSession, subscription: Subscription, subscription_benefit: SubscriptionBenefit, + *, + attempt: int = 1, ) -> SubscriptionBenefitGrant: grant = await self._get_by_subscription_and_benefit( session, subscription, subscription_benefit @@ -32,9 +43,17 @@ async def grant_benefit( benefit_service = get_subscription_benefit_service( subscription_benefit.type, session ) - await benefit_service.grant(subscription_benefit) - - grant.set_granted() + try: + await benefit_service.grant( + subscription_benefit, subscription, attempt=attempt + ) + except SubscriptionBenefitPreconditionError as e: + await self.handle_precondition_error( + session, e, subscription, subscription_benefit + ) + grant.granted_at = None + else: + grant.set_granted() session.add(grant) await session.commit() @@ -46,6 +65,8 @@ async def revoke_benefit( session: AsyncSession, subscription: Subscription, subscription_benefit: SubscriptionBenefit, + *, + attempt: int = 1, ) -> SubscriptionBenefitGrant: grant = await self._get_by_subscription_and_benefit( session, subscription, subscription_benefit @@ -61,7 +82,9 @@ async def revoke_benefit( benefit_service = get_subscription_benefit_service( subscription_benefit.type, session ) - await benefit_service.revoke(subscription_benefit) + await benefit_service.revoke( + subscription_benefit, subscription, attempt=attempt + ) grant.set_revoked() @@ -95,20 +118,31 @@ async def update_benefit_grant( self, session: AsyncSession, grant: SubscriptionBenefitGrant, + *, + attempt: int = 1, ) -> SubscriptionBenefitGrant: # Don't update revoked benefits if grant.is_revoked: return grant - await session.refresh(grant, {"subscription_benefit"}) + await session.refresh(grant, {"subscription", "subscription_benefit"}) + subscription = grant.subscription subscription_benefit = grant.subscription_benefit benefit_service = get_subscription_benefit_service( subscription_benefit.type, session ) - await benefit_service.grant(subscription_benefit) - - grant.set_granted() + try: + await benefit_service.grant( + subscription_benefit, subscription, attempt=attempt + ) + except SubscriptionBenefitPreconditionError as e: + await self.handle_precondition_error( + session, e, subscription, subscription_benefit + ) + grant.granted_at = None + else: + grant.set_granted() session.add(grant) await session.commit() @@ -129,18 +163,23 @@ async def delete_benefit_grant( self, session: AsyncSession, grant: SubscriptionBenefitGrant, + *, + attempt: int = 1, ) -> SubscriptionBenefitGrant: # Already revoked, nothing to do if grant.is_revoked: return grant - await session.refresh(grant, {"subscription_benefit"}) + await session.refresh(grant, {"subscription", "subscription_benefit"}) + subscription = grant.subscription subscription_benefit = grant.subscription_benefit benefit_service = get_subscription_benefit_service( subscription_benefit.type, session ) - await benefit_service.revoke(subscription_benefit) + await benefit_service.revoke( + subscription_benefit, subscription, attempt=attempt + ) grant.set_revoked() @@ -149,6 +188,41 @@ async def delete_benefit_grant( return grant + async def handle_precondition_error( + self, + session: AsyncSession, + error: SubscriptionBenefitPreconditionError, + subscription: Subscription, + subscription_benefit: SubscriptionBenefit, + ) -> None: + if error.email_subject is None or error.email_body_template is None: + log.warning( + "A precondition error was raised but the user was not notified. " + "We probably should implement an email for this error.", + subscription_id=str(subscription.id), + subscription_benefit_id=str(subscription_benefit.id), + ) + return + + email_renderer = get_email_renderer({"subscription": "polar.subscription"}) + email_sender = get_email_sender() + + await session.refresh(subscription, {"user", "subscription_tier"}) + + subject, body = email_renderer.render_from_template( + error.email_subject, + f"subscription/{error.email_body_template}", + { + "subscription": subscription, + "subscription_tier": subscription.subscription_tier, + "subscription_benefit": subscription_benefit, + "user": subscription.user, + **error.email_extra_context, + }, + ) + + email_sender.send_to_user(subscription.user.email, subject, body) + async def _get_by_subscription_and_benefit( self, session: AsyncSession, diff --git a/server/polar/subscription/tasks.py b/server/polar/subscription/tasks.py index 3995543212..71603e7a93 100644 --- a/server/polar/subscription/tasks.py +++ b/server/polar/subscription/tasks.py @@ -1,8 +1,11 @@ import uuid +from arq import Retry + from polar.exceptions import PolarError from polar.worker import AsyncSessionMaker, JobContext, PolarWorkerContext, task +from .service.benefits import SubscriptionBenefitRetriableError from .service.subscription import subscription as subscription_service from .service.subscription_benefit import ( subscription_benefit as subscription_benefit_service, @@ -73,9 +76,12 @@ async def subscription_benefit_grant( if subscription_benefit is None: raise SubscriptionBenefitDoesNotExist(subscription_benefit_id) - await subscription_benefit_grant_service.grant_benefit( - session, subscription, subscription_benefit - ) + try: + await subscription_benefit_grant_service.grant_benefit( + session, subscription, subscription_benefit, attempt=ctx["job_try"] + ) + except SubscriptionBenefitRetriableError as e: + raise Retry(e.defer_seconds) from e @task("subscription.subscription_benefit.revoke") @@ -96,9 +102,12 @@ async def subscription_benefit_revoke( if subscription_benefit is None: raise SubscriptionBenefitDoesNotExist(subscription_benefit_id) - await subscription_benefit_grant_service.revoke_benefit( - session, subscription, subscription_benefit - ) + try: + await subscription_benefit_grant_service.revoke_benefit( + session, subscription, subscription_benefit, attempt=ctx["job_try"] + ) + except SubscriptionBenefitRetriableError as e: + raise Retry(e.defer_seconds) from e @task("subscription.subscription_benefit.update") @@ -114,9 +123,12 @@ async def subscription_benefit_update( if subscription_benefit_grant is None: raise SubscriptionBenefitGrantDoesNotExist(subscription_benefit_grant_id) - await subscription_benefit_grant_service.update_benefit_grant( - session, subscription_benefit_grant - ) + try: + await subscription_benefit_grant_service.update_benefit_grant( + session, subscription_benefit_grant, attempt=ctx["job_try"] + ) + except SubscriptionBenefitRetriableError as e: + raise Retry(e.defer_seconds) from e @task("subscription.subscription_benefit.delete") @@ -132,6 +144,9 @@ async def subscription_benefit_delete( if subscription_benefit_grant is None: raise SubscriptionBenefitGrantDoesNotExist(subscription_benefit_grant_id) - await subscription_benefit_grant_service.delete_benefit_grant( - session, subscription_benefit_grant - ) + try: + await subscription_benefit_grant_service.delete_benefit_grant( + session, subscription_benefit_grant, attempt=ctx["job_try"] + ) + except SubscriptionBenefitRetriableError as e: + raise Retry(e.defer_seconds) from e diff --git a/server/tests/subscription/service/test_subscription_benefit_grant.py b/server/tests/subscription/service/test_subscription_benefit_grant.py index 9aeba2be41..503f5ec4bd 100644 --- a/server/tests/subscription/service/test_subscription_benefit_grant.py +++ b/server/tests/subscription/service/test_subscription_benefit_grant.py @@ -7,7 +7,10 @@ from polar.models import Subscription, SubscriptionBenefit, SubscriptionBenefitGrant from polar.postgres import AsyncSession from polar.subscription.schemas import SubscriptionBenefitCustomUpdate -from polar.subscription.service.benefits import SubscriptionBenefitServiceProtocol +from polar.subscription.service.benefits import ( + SubscriptionBenefitPreconditionError, + SubscriptionBenefitServiceProtocol, +) from polar.subscription.service.subscription_benefit_grant import ( subscription_benefit_grant as subscription_benefit_grant_service, ) @@ -87,6 +90,23 @@ async def test_existing_grant_already_granted( assert updated_grant.is_granted subscription_benefit_service_mock.grant.assert_not_called() + async def test_precondition_error( + self, + session: AsyncSession, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + subscription_benefit_service_mock: MagicMock, + ) -> None: + subscription_benefit_service_mock.grant.side_effect = ( + SubscriptionBenefitPreconditionError("Error") + ) + + grant = await subscription_benefit_grant_service.grant_benefit( + session, subscription, subscription_benefit_organization + ) + + assert not grant.is_granted + @pytest.mark.asyncio class TestRevokeBenefit: @@ -270,6 +290,31 @@ async def test_granted_grant( assert updated_grant.is_granted subscription_benefit_service_mock.grant.assert_called_once() + async def test_precondition_error( + self, + session: AsyncSession, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + subscription_benefit_service_mock: MagicMock, + ) -> None: + grant = SubscriptionBenefitGrant( + subscription=subscription, + subscription_benefit=subscription_benefit_organization, + ) + grant.set_granted() + session.add(grant) + await session.commit() + + subscription_benefit_service_mock.grant.side_effect = ( + SubscriptionBenefitPreconditionError("Error") + ) + + updated_grant = await subscription_benefit_grant_service.update_benefit_grant( + session, grant + ) + + assert not updated_grant.is_granted + @pytest.mark.asyncio class TestEnqueueBenefitGrantDeletions: @@ -364,3 +409,55 @@ async def test_granted_grant( assert updated_grant.id == grant.id assert updated_grant.is_revoked subscription_benefit_service_mock.revoke.assert_called_once() + + +@pytest.fixture +def email_sender_mock(mocker: MockerFixture) -> MagicMock: + email_sender_mock = MagicMock() + mocker.patch( + "polar.subscription.service.subscription_benefit_grant.get_email_sender", + return_value=email_sender_mock, + ) + return email_sender_mock + + +@pytest.mark.asyncio +class TestHandlePreconditionError: + async def test_no_email( + self, + session: AsyncSession, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + email_sender_mock: MagicMock, + ) -> None: + error = SubscriptionBenefitPreconditionError("Error") + + await subscription_benefit_grant_service.handle_precondition_error( + session, error, subscription, subscription_benefit_organization + ) + + email_sender_mock.assert_not_called() + + async def test_email( + self, + session: AsyncSession, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + email_sender_mock: MagicMock, + ) -> None: + error = SubscriptionBenefitPreconditionError( + "Error", + email_subject="Email subject", + email_body_template="benefits/custom/precondition_failed.html", + email_extra_context={"foo": "bar"}, + ) + + await subscription_benefit_grant_service.handle_precondition_error( + session, error, subscription, subscription_benefit_organization + ) + + send_to_user_mock: MagicMock = email_sender_mock.send_to_user + assert send_to_user_mock.called + to_email_addr = send_to_user_mock.call_args[0][0] + + assert subscription.user.email == to_email_addr diff --git a/server/tests/subscription/test_tasks.py b/server/tests/subscription/test_tasks.py index f93d5105f3..3c303888ea 100644 --- a/server/tests/subscription/test_tasks.py +++ b/server/tests/subscription/test_tasks.py @@ -1,10 +1,12 @@ import uuid import pytest +from arq import Retry from pytest_mock import MockerFixture from polar.models import Subscription, SubscriptionBenefit, SubscriptionBenefitGrant from polar.postgres import AsyncSession +from polar.subscription.service.benefits import SubscriptionBenefitRetriableError from polar.subscription.service.subscription import SubscriptionService from polar.subscription.service.subscription_benefit_grant import ( SubscriptionBenefitGrantService, @@ -104,6 +106,29 @@ async def test_existing_subscription_and_benefit( grant_benefit_mock.assert_called_once() + async def test_retry( + self, + mocker: MockerFixture, + job_context: JobContext, + polar_worker_context: PolarWorkerContext, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + ) -> None: + grant_benefit_mock = mocker.patch.object( + subscription_benefit_grant_service, + "grant_benefit", + spec=SubscriptionBenefitGrantService.grant_benefit, + ) + grant_benefit_mock.side_effect = SubscriptionBenefitRetriableError(10) + + with pytest.raises(Retry): + await subscription_benefit_grant( + job_context, + subscription.id, + subscription_benefit_organization.id, + polar_worker_context, + ) + @pytest.mark.asyncio class TestSubscriptionBenefitRevoke: @@ -155,6 +180,29 @@ async def test_existing_subscription_and_benefit( revoke_benefit_mock.assert_called_once() + async def test_retry( + self, + mocker: MockerFixture, + job_context: JobContext, + polar_worker_context: PolarWorkerContext, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + ) -> None: + revoke_benefit_mock = mocker.patch.object( + subscription_benefit_grant_service, + "revoke_benefit", + spec=SubscriptionBenefitGrantService.revoke_benefit, + ) + revoke_benefit_mock.side_effect = SubscriptionBenefitRetriableError(10) + + with pytest.raises(Retry): + await subscription_benefit_revoke( + job_context, + subscription.id, + subscription_benefit_organization.id, + polar_worker_context, + ) + @pytest.mark.asyncio class TestSubscriptionBenefitUpdate: @@ -196,6 +244,35 @@ async def test_existing_grant( update_benefit_grant_mock.assert_called_once() + async def test_retry( + self, + session: AsyncSession, + mocker: MockerFixture, + job_context: JobContext, + polar_worker_context: PolarWorkerContext, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + ) -> None: + grant = SubscriptionBenefitGrant( + subscription=subscription, + subscription_benefit=subscription_benefit_organization, + ) + grant.set_granted() + session.add(grant) + await session.commit() + + update_benefit_grant_mock = mocker.patch.object( + subscription_benefit_grant_service, + "update_benefit_grant", + spec=SubscriptionBenefitGrantService.update_benefit_grant, + ) + update_benefit_grant_mock.side_effect = SubscriptionBenefitRetriableError(10) + + with pytest.raises(Retry): + await subscription_benefit_update( + job_context, grant.id, polar_worker_context + ) + @pytest.mark.asyncio class TestSubscriptionBenefitDelete: @@ -236,3 +313,32 @@ async def test_existing_grant( await subscription_benefit_delete(job_context, grant.id, polar_worker_context) delete_benefit_grant_mock.assert_called_once() + + async def test_retry( + self, + session: AsyncSession, + mocker: MockerFixture, + job_context: JobContext, + polar_worker_context: PolarWorkerContext, + subscription: Subscription, + subscription_benefit_organization: SubscriptionBenefit, + ) -> None: + grant = SubscriptionBenefitGrant( + subscription=subscription, + subscription_benefit=subscription_benefit_organization, + ) + grant.set_granted() + session.add(grant) + await session.commit() + + delete_benefit_grant_mock = mocker.patch.object( + subscription_benefit_grant_service, + "delete_benefit_grant", + spec=SubscriptionBenefitGrantService.delete_benefit_grant, + ) + delete_benefit_grant_mock.side_effect = SubscriptionBenefitRetriableError(10) + + with pytest.raises(Retry): + await subscription_benefit_delete( + job_context, grant.id, polar_worker_context + )