From ed658c4dfdebf8a1ea18bcfc1c3844923cbd5519 Mon Sep 17 00:00:00 2001 From: Sonny Date: Mon, 1 Jul 2019 11:38:23 +0200 Subject: [PATCH] Handle request exceptions --- src/newsreader/news/collection/exceptions.py | 4 ++ .../news/collection/response_handler.py | 33 +++++++++--- .../news/collection/tests/utils/tests.py | 50 +++++++++++++++++++ src/newsreader/news/collection/utils.py | 15 +++--- 4 files changed, 88 insertions(+), 14 deletions(-) diff --git a/src/newsreader/news/collection/exceptions.py b/src/newsreader/news/collection/exceptions.py index 8e12da1..e636638 100644 --- a/src/newsreader/news/collection/exceptions.py +++ b/src/newsreader/news/collection/exceptions.py @@ -26,3 +26,7 @@ class StreamForbiddenException(StreamException): class StreamParseException(StreamException): message = "Stream could not be parsed" + + +class StreamConnectionError(StreamException): + message = "A connection to the stream could not be made" diff --git a/src/newsreader/news/collection/response_handler.py b/src/newsreader/news/collection/response_handler.py index ff46171..e412475 100644 --- a/src/newsreader/news/collection/response_handler.py +++ b/src/newsreader/news/collection/response_handler.py @@ -1,9 +1,18 @@ from typing import ContextManager from requests import Response +from requests.exceptions import ConnectionError as RequestConnectionError +from requests.exceptions import ( + HTTPError, + RequestException, + SSLError, + TooManyRedirects, +) from newsreader.news.collection.exceptions import ( + StreamConnectionError, StreamDeniedException, + StreamException, StreamForbiddenException, StreamNotFoundException, StreamTimeOutException, @@ -11,24 +20,32 @@ from newsreader.news.collection.exceptions import ( class ResponseHandler: - message_mapping = { + status_code_mapping = { 404: StreamNotFoundException, 401: StreamDeniedException, 403: StreamForbiddenException, 408: StreamTimeOutException, } - def __init__(self, response: Response) -> None: - self.response = response + exception_mapping = {RequestConnectionError: StreamConnectionError} def __enter__(self) -> ContextManager: return self - def handle_response(self) -> None: - status_code = self.response.status_code + def handle_response(self, response) -> None: + status_code = response.status_code - if status_code in self.message_mapping: - raise self.message_mapping[status_code] + if status_code in self.status_code_mapping: + raise self.status_code_mapping[status_code] + + def handle_exception(self, exception): + try: + stream_exception = self.exception_mapping[type(exception)] + except KeyError: + stream_exception = StreamException + + message = getattr(exception, "message", str(exception)) + raise stream_exception(message=message) from exception def __exit__(self, *args, **kwargs) -> None: - self.response = None + pass diff --git a/src/newsreader/news/collection/tests/utils/tests.py b/src/newsreader/news/collection/tests/utils/tests.py index 3c95df0..e2f0fcf 100644 --- a/src/newsreader/news/collection/tests/utils/tests.py +++ b/src/newsreader/news/collection/tests/utils/tests.py @@ -1,9 +1,19 @@ from unittest.mock import MagicMock, patch +from requests.exceptions import ConnectionError as RequestConnectionError +from requests.exceptions import ( + HTTPError, + RequestException, + SSLError, + TooManyRedirects, +) + from django.test import TestCase from newsreader.news.collection.exceptions import ( + StreamConnectionError, StreamDeniedException, + StreamException, StreamForbiddenException, StreamNotFoundException, StreamTimeOutException, @@ -55,3 +65,43 @@ class FetchTestCase(TestCase): with self.assertRaises(StreamTimeOutException): fetch(url) + + def test_raises_stream_error_on_ssl_error(self): + self.mocked_get.side_effect = SSLError + + url = "https://www.bbc.co.uk/news" + + with self.assertRaises(StreamException): + fetch(url) + + def test_raises_stream_error_on_connection_error(self): + self.mocked_get.side_effect = RequestConnectionError + + url = "https://www.bbc.co.uk/news" + + with self.assertRaises(StreamConnectionError): + fetch(url) + + def test_raises_stream_error_on_http_error(self): + self.mocked_get.side_effect = HTTPError + + url = "https://www.bbc.co.uk/news" + + with self.assertRaises(StreamException): + fetch(url) + + def test_raises_stream_error_on_request_exception(self): + self.mocked_get.side_effect = RequestException + + url = "https://www.bbc.co.uk/news" + + with self.assertRaises(StreamException): + fetch(url) + + def test_raises_stream_error_on_too_many_redirects(self): + self.mocked_get.side_effect = TooManyRedirects + + url = "https://www.bbc.co.uk/news" + + with self.assertRaises(StreamException): + fetch(url) diff --git a/src/newsreader/news/collection/utils.py b/src/newsreader/news/collection/utils.py index 17f379a..527587d 100644 --- a/src/newsreader/news/collection/utils.py +++ b/src/newsreader/news/collection/utils.py @@ -1,9 +1,10 @@ from datetime import datetime, tzinfo from time import mktime, struct_time -from typing import Optional, Tuple +from typing import Tuple import requests +from requests.exceptions import RequestException from requests.models import Response from django.utils import timezone @@ -20,10 +21,12 @@ def build_publication_date(dt: struct_time, tz: tzinfo) -> Tuple: return True, published_parsed -def fetch(url: str) -> Optional[Response]: - response = requests.get(url) - - with ResponseHandler(response) as response_handler: - response_handler.handle_response() +def fetch(url: str) -> Response: + with ResponseHandler() as response_handler: + try: + response = requests.get(url) + response_handler.handle_response(response) + except RequestException as exception: + response_handler.handle_exception(exception) return response