Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Commit

Permalink
Do not 500 when upstream provider is unavailable
Browse files Browse the repository at this point in the history
  • Loading branch information
sarayourfriend committed Aug 16, 2022
1 parent 2d8cf1f commit 3ea70b3
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 2 deletions.
14 changes: 12 additions & 2 deletions api/catalog/api/views/media_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
from django.http.response import HttpResponse
from rest_framework import status
from rest_framework.decorators import action
from rest_framework.exceptions import APIException
from rest_framework.response import Response
from rest_framework.viewsets import ReadOnlyModelViewSet

from sentry_sdk import capture_exception

from catalog.api.controllers import search_controller
from catalog.api.models import ContentProvider
from catalog.api.serializers.provider_serializers import ProviderSerializer
Expand All @@ -20,6 +23,11 @@
from catalog.custom_auto_schema import CustomAutoSchema


class UpstreamThumbnailException(APIException):
status_code = status.HTTP_424_FAILED_DEPENDENCY
default_detail = "Could not render thumbnail due to upstream provider error."


class MediaViewSet(ReadOnlyModelViewSet):
swagger_schema = CustomAutoSchema

Expand Down Expand Up @@ -189,9 +197,11 @@ def _thumbnail_proxy_comm(

return upstream_response, res_status, content_type
except (HTTPError, RemoteDisconnected, TimeoutError) as exc:
raise get_api_exception(f"Failed to render thumbnail: {exc}")
capture_exception(exc)
raise UpstreamThumbnailException(f"Failed to render thumbnail: {exc}")
except Exception as exc:
raise get_api_exception(
capture_exception(exc)
raise UpstreamThumbnailException(
f"Failed to render thumbnail due to unidentified exception: {exc}"
)

Expand Down
8 changes: 8 additions & 0 deletions api/test/factory/models/image.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from test.factory.models.media import MediaFactory

from catalog.api.models.image import Image


class ImageFactory(MediaFactory):
class Meta:
model = Image
5 changes: 5 additions & 0 deletions api/test/image_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
stats,
thumb,
thumb_compression,
thumb_error,
thumb_full_size,
thumb_webp,
uuid_validation,
Expand Down Expand Up @@ -90,6 +91,10 @@ def test_image_thumb_full_size(image_fixture):
thumb_full_size(image_fixture)


def test_image_thumb_failure(image_fixture):
thumb_error(image_fixture)


def test_audio_report(image_fixture):
report("images", image_fixture)

Expand Down
40 changes: 40 additions & 0 deletions api/test/unit/views/media_views_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
from test.factory.models.image import ImageFactory
from unittest import mock
from urllib.error import HTTPError

from rest_framework.test import APIClient

import pytest

from catalog.api.models.image import Image


@pytest.fixture
def api_client() -> APIClient:
return APIClient()


@pytest.fixture
def image() -> Image:
return ImageFactory.create()


@pytest.mark.django_db
def test_thumb_error(api_client, image):
error = None

def urlopen_503_response(url, **kwargs):
nonlocal error
error = HTTPError(url, 503, "Bad error upstream whoops", {}, None)
raise error

with mock.patch(
"catalog.api.views.media_views.urlopen"
) as urlopen_mock, mock.patch(
"catalog.api.views.media_views.capture_exception", autospec=True
) as mock_capture_exception:
urlopen_mock.side_effect = urlopen_503_response
response = api_client.get(f"/v1/images/{image.identifier}/thumb/")

assert response.status_code == 424
mock_capture_exception.assert_called_once_with(error)

0 comments on commit 3ea70b3

Please sign in to comment.