Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add more semantics to API exceptions related to subsidy fulfillment #207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 143 additions & 0 deletions enterprise_access/apps/api/v1/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
"""
Version 1 API Exceptions.
"""
from rest_framework import status
from rest_framework.exceptions import APIException

from enterprise_access.apps.api_client.lms_client import LmsApiClient
from enterprise_access.apps.subsidy_access_policy import constants


class RedemptionRequestException(APIException):
status_code = status.HTTP_422_UNPROCESSABLE_ENTITY
default_detail = 'Could not redeem'


class SubsidyAPIRedemptionRequestException(RedemptionRequestException):
"""
An API exception that has a response payload structured like
{
'code': 'some_error_code',
'detail': {
'reason': 'reason_for_error',
'user_message': 'User friendly string describing the error.',
# additional metadata describing the error, possibly including admin emails.
'metadata': {
'key': 'value',
}
}
}

There are some sane defaults set at initialization for the reason, code, and user_message
values.
"""
default_detail = 'Error redeeming through Subsidy API'
default_code = constants.SubsidyRedemptionErrorCodes.DEFAULT_ERROR

# Custom keys of the `detail` field returned in the response payload.
default_reason = constants.SubsidyRedemptionErrorReasons.DEFAULT_REASON
default_user_message = constants.SubsidyRedemptionErrorReasons.USER_MESSAGES_BY_REASON[default_reason]

def __init__(self, code=None, detail=None, policy=None, subsidy_api_error=None):
"""
Initializes all of the attributes of this exception instance.

args:
code (str): A reusable error code constant.
detail ([list,str,dict]): Details about the exception we're raising.
policy (SubsidyAccessPolicy): A policy object, from which we can fetch admin email addresses.
subsidy_api_error (SubsidyAPIHTTPError): The exception object that was caught, from which
we can infer more specific causes about the redemption error this exception encapsulates.
"""
super().__init__(code=code, detail=detail)

self.reason = self.default_reason
self.user_message = self.default_user_message
self.metadata = {}

if policy and subsidy_api_error:
try:
self._build_subsidy_api_error_payload(policy, subsidy_api_error)
except Exception: # pylint: disable=broad-except
self.metadata = {
'subsidy_error_detail_raw': str(subsidy_api_error.error_response.content),
}

self.detail = {
'code': self.code,
'detail': {
'reason': self.reason,
'user_message': self.user_message,
'metadata': self.metadata,
}
}

def _build_subsidy_api_error_payload(self, policy, subsidy_api_error):
"""
Helper to build error response payload on Subsidy API errors.
"""
subsidy_error_detail = subsidy_api_error.error_payload().get('detail')
subsidy_error_code = subsidy_api_error.error_payload().get('code')

self.metadata = {
'enterprise_administrators': LmsApiClient().get_enterprise_customer_data(
policy.enterprise_customer_uuid
).get('admin_users')
}

# We currently only have enough data to say more specific things
# about fulfillment errors during subsidy API redemption.
if subsidy_error_code == constants.SubsidyRedemptionErrorCodes.FULFILLMENT_ERROR:
self._set_subsidy_fulfillment_error_reason(subsidy_error_detail)

def _set_subsidy_fulfillment_error_reason(self, subsidy_error_detail):
"""
Helper to set the reason, user_message, and metadata
for the given subsidy_error_detail.
"""
self.metadata['subsidy_error_detail'] = subsidy_error_detail
self.reason = constants.SubsidyFulfillmentErrorReasons.DEFAULT_REASON

if subsidy_error_detail:
message_string = self._get_subsidy_fulfillment_error_message(subsidy_error_detail)
if cause_of_message := constants.SubsidyFulfillmentErrorReasons.get_cause_from_error_message(
message_string
):
self.reason = cause_of_message
# pylint: disable=attribute-defined-outside-init
self.code = constants.SubsidyRedemptionErrorCodes.FULFILLMENT_ERROR

self.user_message = constants.SubsidyFulfillmentErrorReasons.USER_MESSAGES_BY_REASON.get(self.reason)

def _get_subsidy_fulfillment_error_message(self, subsidy_error_detail):
"""
``subsidy_error_detail`` is either a string describing an error message,
a dict with a "message" key describing an error message, or a list of such
dicts. This helper method widdles any of those things down into a single
error message string.
Comment on lines +114 to +117
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just the result of the subsidy API being indecisive about error formatting and being written by >1 person?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof yes, can we fix the offending call?

Copy link
Contributor Author

@iloveagent57 iloveagent57 Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it to be defensive about the types of messages that can exist from a DRF APIException: https://github.com/encode/django-rest-framework/blob/master/rest_framework/exceptions.py

"""
if isinstance(subsidy_error_detail, str):
return subsidy_error_detail

subsidy_message_dict = subsidy_error_detail
if isinstance(subsidy_error_detail, list):
subsidy_message_dict = subsidy_error_detail[0]

return subsidy_message_dict.get('message')


class SubsidyAccessPolicyLockedException(APIException):
"""
Throw this exception when an attempt to acquire a policy lock failed because it was already locked by another agent.

Note: status.HTTP_423_LOCKED is NOT acceptable as a status code for delivery to web browsers. According to Mozilla:

> The ability to lock a resource is specific to some WebDAV servers. Browsers accessing web pages will never
> encounter this status code; in the erroneous cases it happens, they will handle it as a generic 400 status code.

See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/423

HTTP 429 Too Many Requests is the next best thing, and implies retryability.
"""
status_code = status.HTTP_429_TOO_MANY_REQUESTS
default_detail = 'Enrollment currently locked for this subsidy access policy.'
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from uuid import UUID, uuid4

import ddt
import requests
from django.conf import settings
from rest_framework import status
from rest_framework.reverse import reverse
Expand All @@ -22,6 +23,8 @@
AccessMethods,
MissingSubsidyAccessReasonUserMessages,
PolicyTypes,
SubsidyFulfillmentErrorReasons,
SubsidyRedemptionErrorCodes,
TransactionStateChoices
)
from enterprise_access.apps.subsidy_access_policy.tests.factories import (
Expand Down Expand Up @@ -682,6 +685,65 @@ def test_redeem_policy(self, mock_transactions_cache_for_learner): # pylint: di
),
)

@mock.patch('enterprise_access.apps.subsidy_access_policy.models.get_and_cache_transactions_for_learner')
@mock.patch('enterprise_access.apps.api.v1.exceptions.LmsApiClient')
@ddt.data(
{
'subsidy_error_code': 'fulfillment_error',
'subsidy_error_detail': [
{'message': 'woozit duplicate order woohoo!'},
],
'expected_redeem_error_detail': {
'reason': SubsidyFulfillmentErrorReasons.DUPLICATE_FULFILLMENT,
'user_message': SubsidyFulfillmentErrorReasons.USER_MESSAGES_BY_REASON[
SubsidyFulfillmentErrorReasons.DUPLICATE_FULFILLMENT
],
'metadata': {
'enterprise_administrators': [{'email': 'edx@example.com'}],
'subsidy_error_detail': [
{'message': 'woozit duplicate order woohoo!'}
],
},
},
'expected_redeem_error_code': SubsidyRedemptionErrorCodes.FULFILLMENT_ERROR,
},
)
@ddt.unpack
def test_redeem_policy_subsidy_api_error(
self, mock_lms_api_client, mock_transactions_cache_for_learner, # pylint: disable=unused-argument
subsidy_error_code, subsidy_error_detail,
expected_redeem_error_detail, expected_redeem_error_code
):
"""
Verify that SubsidyAccessPolicyRedeemViewset redeem endpoint returns a well-structured
error response payload when the subsidy API call to redeem/fulfill responds with an error.
"""
mock_lms_api_client().get_enterprise_customer_data.return_value = {
'slug': 'the-slug',
'admin_users': [{'email': 'edx@example.com'}],
}
self.mock_get_content_metadata.return_value = {'content_price': 123}
mock_response = mock.MagicMock()
mock_response.json.return_value = {
'code': subsidy_error_code,
'detail': subsidy_error_detail,
}
self.redeemable_policy.subsidy_client.create_subsidy_transaction.side_effect = requests.exceptions.HTTPError(
response=mock_response
)

payload = {
'lms_user_id': 1234,
'content_key': 'course-v1:edX+edXPrivacy101+3T2020',
}

response = self.client.post(self.subsidy_access_policy_redeem_endpoint, payload)

response_json = self.load_json(response.content)
self.maxDiff = None
self.assertEqual(response_json['detail'], expected_redeem_error_detail)
self.assertEqual(response_json['code'], expected_redeem_error_code)

@mock.patch('enterprise_access.apps.subsidy_access_policy.models.get_and_cache_transactions_for_learner')
def test_redeem_policy_with_metadata(self, mock_transactions_cache_for_learner): # pylint: disable=unused-argument
"""
Expand Down
50 changes: 19 additions & 31 deletions enterprise_access/apps/api/v1/views/subsidy_access_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from rest_framework import serializers as rest_serializers
from rest_framework import status, viewsets
from rest_framework.decorators import action
from rest_framework.exceptions import APIException, NotFound
from rest_framework.exceptions import NotFound
from rest_framework.generics import get_object_or_404
from rest_framework.response import Response

Expand Down Expand Up @@ -57,6 +57,11 @@
)
from enterprise_access.apps.subsidy_access_policy.subsidy_api import get_redemptions_by_content_and_policy_for_learner

from ..exceptions import (
RedemptionRequestException,
SubsidyAccessPolicyLockedException,
SubsidyAPIRedemptionRequestException
)
from .utils import PaginationWithPageCount

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -244,28 +249,6 @@ def get_queryset(self):
return queryset.filter(enterprise_customer_uuid=enterprise_customer_uuid)


class RedemptionRequestException(APIException):
status_code = status.HTTP_422_UNPROCESSABLE_ENTITY
default_detail = 'Could not redeem'


class SubsidyAccessPolicyLockedException(APIException):
"""
Throw this exception when an attempt to acquire a policy lock failed because it was already locked by another agent.

Note: status.HTTP_423_LOCKED is NOT acceptable as a status code for delivery to web browsers. According to Mozilla:

> The ability to lock a resource is specific to some WebDAV servers. Browsers accessing web pages will never
> encounter this status code; in the erroneous cases it happens, they will handle it as a generic 400 status code.

See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/423

HTTP 429 Too Many Requests is the next best thing, and implies retryability.
"""
status_code = status.HTTP_429_TOO_MANY_REQUESTS
default_detail = 'Enrollment currently locked for this subsidy access policy.'


class SubsidyAccessPolicyRedeemViewset(UserDetailsFromJwtMixin, PermissionRequiredMixin, viewsets.GenericViewSet):
"""
Viewset for Subsidy Access Policy APIs.
Expand Down Expand Up @@ -484,11 +467,10 @@ def redeem(self, request, *args, **kwargs):
logger.exception(exc)
raise SubsidyAccessPolicyLockedException() from exc
except SubsidyAPIHTTPError as exc:
logger.exception(f'{exc} when creating transaction in subsidy API')
error_payload = exc.error_payload()
error_payload['detail'] = f"Subsidy Transaction API error: {error_payload['detail']}"
raise RedemptionRequestException(
detail=error_payload,
logger.exception(f'{exc} when creating transaction in subsidy API with payload {exc.error_payload()}')
raise SubsidyAPIRedemptionRequestException(
policy=policy,
subsidy_api_error=exc,
) from exc

def get_existing_redemptions(self, policies, lms_user_id):
Expand Down Expand Up @@ -673,9 +655,7 @@ def _get_reasons_for_no_redeemable_policies(self, enterprise_customer_uuid, non_
for each non-redeemable policy.
"""
reasons = []
lms_client = LmsApiClient()
enterprise_customer_data = lms_client.get_enterprise_customer_data(enterprise_customer_uuid)
enterprise_admin_users = enterprise_customer_data.get('admin_users')
enterprise_admin_users = self._get_enterprise_admin_users(enterprise_customer_uuid)

for reason, policies in non_redeemable_policies.items():
reasons.append({
Expand All @@ -689,6 +669,14 @@ def _get_reasons_for_no_redeemable_policies(self, enterprise_customer_uuid, non_

return reasons

def _get_enterprise_admin_users(self, enterprise_customer_uuid):
"""
Helper to fetch admin users for the given customer uuid.
"""
lms_client = LmsApiClient()
enterprise_customer_data = lms_client.get_enterprise_customer_data(enterprise_customer_uuid)
return enterprise_customer_data.get('admin_users')

def _get_list_price(self, enterprise_customer_uuid, content_key):
"""
Determine the price for content for display purposes only.
Expand Down
55 changes: 55 additions & 0 deletions enterprise_access/apps/subsidy_access_policy/constants.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
""" Constants for the subsidy_access_policy app. """
import re


class AccessMethods:
Expand Down Expand Up @@ -99,3 +100,57 @@ class MissingSubsidyAccessReasonUserMessages:
REASON_LEARNER_MAX_SPEND_REACHED = "learner_max_spend_reached"
REASON_POLICY_SPEND_LIMIT_REACHED = "policy_spend_limit_reached"
REASON_LEARNER_MAX_ENROLLMENTS_REACHED = "learner_max_enrollments_reached"


class SubsidyRedemptionErrorCodes:
"""
Collection of error ``code`` values that the subsidy API's
redeem endpoint might return in an error response payload.
"""
DEFAULT_ERROR = 'subsidy_redemption_error'
FULFILLMENT_ERROR = 'fulfillment_error'


class SubsidyRedemptionErrorReasons:
"""
Somewhat more generic collection of reasons that redemption may have
failed in ways that are *not* related to fulfillment.
"""
DEFAULT_REASON = 'default_subsidy_redemption_error'

USER_MESSAGES_BY_REASON = {
DEFAULT_REASON: "Something went wrong during subsidy redemption",
}


class SubsidyFulfillmentErrorReasons:
"""
Codifies standard reasons that fulfillment may have failed,
along with a mapping of those reasons to user-friendly display messages.
"""
DEFAULT_REASON = 'default_fulfillment_error'
DUPLICATE_FULFILLMENT = 'duplicate_fulfillment'

USER_MESSAGES_BY_REASON = {
DEFAULT_REASON: "Something went wrong during fulfillment",
DUPLICATE_FULFILLMENT: "A legacy fulfillment already exists for this content.",
}

CAUSES_REGEXP_BY_REASON = {
DUPLICATE_FULFILLMENT: re.compile(".*duplicate order.*"),
}

@classmethod
def get_cause_from_error_message(cls, message_string):
"""
Helper to find the cause of a given error message string
by matching against the regexs mapped above.
"""
if not message_string:
return None

for cause_of_message, regex in cls.CAUSES_REGEXP_BY_REASON.items():
if regex.match(message_string):
return cause_of_message

return None