Skip to content
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
59 changes: 57 additions & 2 deletions course_discovery/apps/course_metadata/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import responses
from django.conf import settings
from django.core.exceptions import ValidationError
from django.http.response import HttpResponse
from django.test import TestCase
from edx_django_utils.cache import RequestCache
from edx_toggles.toggles.testutils import override_waffle_switch
from slugify import slugify
from slumber.exceptions import HttpClientError

from course_discovery.apps.api.tests.mixins import SiteMixin
from course_discovery.apps.api.v1.tests.test_views.mixins import OAuth2Mixin
Expand Down Expand Up @@ -43,8 +45,8 @@
from course_discovery.apps.course_metadata.utils import (
calculated_seat_upgrade_deadline, clean_html, convert_svg_to_png_from_url, create_missing_entitlement,
download_and_save_course_image, download_and_save_program_image, ensure_draft_world, fetch_getsmarter_products,
generate_sku, is_google_drive_url, serialize_entitlement_for_ecommerce_api, serialize_seat_for_ecommerce_api,
transform_skills_data, validate_slug_format
generate_sku, is_fatal_error, is_google_drive_url, serialize_entitlement_for_ecommerce_api,
serialize_seat_for_ecommerce_api, transform_skills_data, validate_slug_format
)


Expand Down Expand Up @@ -1161,6 +1163,59 @@ def tearDown(self):
responses.reset()
super().tearDown()

def test_is_fatal_code(self):
response_with_200 = HttpResponse(status=200)
response_with_400 = HttpResponse(status=400)
response_with_429 = HttpResponse(status=429)
response_with_504 = HttpResponse(status=504)
assert not is_fatal_error(HttpClientError(response=response_with_200))
assert is_fatal_error(HttpClientError(response=response_with_400))
assert not is_fatal_error(HttpClientError(response=response_with_429))
assert not is_fatal_error(HttpClientError(response=response_with_504))

def test_is_fatal_code_error(self):
# Success codes (2xx) should not be fatal error
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=201)))
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=204)))

# Redirect codes (3xx) should not be fatal error
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=301)))
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=302)))
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=307)))

# Retryable client error (429 Too Many Requests) already covered
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=429)))

# Retryable server errors (5xx) should not be fatal
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=500)))
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=502)))
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=503)))
assert not is_fatal_error(HttpClientError(response=HttpResponse(status=504)))

def test_is_fatal_code_positive(self):
# Client errors (4xx) that should be fatal
assert is_fatal_error(HttpClientError(response=HttpResponse(status=400))) # Bad Request
assert is_fatal_error(HttpClientError(response=HttpResponse(status=401))) # Unauthorized
assert is_fatal_error(HttpClientError(response=HttpResponse(status=403))) # Forbidden
assert is_fatal_error(HttpClientError(response=HttpResponse(status=404))) # Not Found

# Other 4xx codes
assert is_fatal_error(HttpClientError(response=HttpResponse(status=410))) # Gone
assert is_fatal_error(HttpClientError(response=HttpResponse(status=422))) # Unprocessable Entity

def test_is_fatal_code_additional(self):
# More client errors (4xx) that should be fatal
assert is_fatal_error(HttpClientError(response=HttpResponse(status=405))) # Method Not Allowed
assert is_fatal_error(HttpClientError(response=HttpResponse(status=406))) # Not Acceptable
assert is_fatal_error(HttpClientError(response=HttpResponse(status=407))) # Proxy Authentication Required
assert is_fatal_error(HttpClientError(response=HttpResponse(status=408))) # Request Timeout
assert is_fatal_error(HttpClientError(response=HttpResponse(status=409))) # Conflict

# Validation-related errors
assert is_fatal_error(HttpClientError(response=HttpResponse(status=415))) # Unsupported Media Type
assert is_fatal_error(HttpClientError(response=HttpResponse(status=417))) # Expectation Failed
assert is_fatal_error(HttpClientError(response=HttpResponse(status=451))) # Unavailable For Legal Reason

def mock_product_api_call(self):
"""
Mock product api with success response.
Expand Down
34 changes: 34 additions & 0 deletions course_discovery/apps/course_metadata/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from tempfile import NamedTemporaryFile
from urllib.parse import urljoin, urlparse

import backoff
import html2text
import jsonschema
import markdown
Expand Down Expand Up @@ -73,6 +74,26 @@ def clean_query(query):
return query


def is_fatal_error(ex):
"""
Return True if the exception represents a fatal client error.

Fatal means:
- The response exists
- The status code is a 4XX client error (400–499)
- The error is not a 429 (rate limiting)
"""
response = ex.response
if response is None:
return False

code = response.status_code
if code == 429:
return False

return 400 <= code < 500


def set_official_state(obj, model, attrs=None):
"""
Given a draft object and the model of that object, ensure that an official version is created
Expand Down Expand Up @@ -986,6 +1007,19 @@ def transform_skills_data(skills_data):
return skills


# The courses endpoint has 40 requests/minute rate limit.
# This will back off at a rate of 60/120/240 seconds (from the factor 60 and default value of base 2).
# This backoff code can still fail because of the concurrent requests all requesting at the same time.
# So even in the case of entering into the next minute, if we still exceed our limit for that min,
# any requests that failed in both limits are still approaching their max_tries limit.
@backoff.on_exception(
backoff.expo,
(requests.exceptions.Timeout, requests.exceptions.HTTPError),
factor=60,
max_tries=4,
giveup=is_fatal_error,
max_time=300
)
def fetch_getsmarter_products():
""" Returns the products details from the getsmarter API """
products = []
Expand Down
Loading