From 06bc6fadb787f6ff7f41f8d97c0415f4e74038e6 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 13:38:57 +0000 Subject: [PATCH 01/10] Raise for status when Yahoo! returns 403 Sometimes Yahoo! returns 403. This causes a tuple destructuring error later in `parse_response()`, when `.json()` actually does not return anything that can be destructured. The correct defensive behavior is to fail fast and exit here. --- beanprice/sources/yahoo.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beanprice/sources/yahoo.py b/beanprice/sources/yahoo.py index 64c8b2b..cb7e207 100644 --- a/beanprice/sources/yahoo.py +++ b/beanprice/sources/yahoo.py @@ -86,6 +86,7 @@ def get_price_series(ticker: str, } payload.update(_DEFAULT_PARAMS) response = requests.get(url, params=payload, headers={'User-Agent': None}) + response.raise_for_status() result = parse_response(response) meta = result['meta'] @@ -121,6 +122,7 @@ def get_latest_price(self, ticker: str) -> Optional[source.SourcePrice]: } payload.update(_DEFAULT_PARAMS) response = requests.get(url, params=payload, headers={'User-Agent': None}) + response.raise_for_status() try: result = parse_response(response) except YahooError as error: From 0261e1fca7ea216ea33b8e8c2be8bd68b6ba1d31 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 13:46:03 +0000 Subject: [PATCH 02/10] Make other fixes. --- beanprice/sources/yahoo.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/beanprice/sources/yahoo.py b/beanprice/sources/yahoo.py index cb7e207..aec9751 100644 --- a/beanprice/sources/yahoo.py +++ b/beanprice/sources/yahoo.py @@ -29,6 +29,16 @@ class YahooError(ValueError): "An error from the Yahoo API." +def _requestor(*args, **kwargs): + if "headers" not in kwargs: + kwargs["headers"] = {} + # Yahoo! balks without this header. + kwargs["headers"]["User-Agent"] = "Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/110.0" + response = requests.get(*args, **kwargs) + response.raise_for_status() + return response + + def parse_response(response: requests.models.Response) -> Dict: """Process as response from Yahoo. @@ -44,16 +54,16 @@ def parse_response(response: requests.models.Response) -> Dict: ','.join(json.keys()))) if content['error'] is not None: raise YahooError("Error fetching Yahoo data: {}".format(content['error'])) - if not content['result']: - raise YahooError("No data returned from Yahoo, ensure that the symbol is correct") - return content['result'][0] + try: + return content['result'][0] + except IndexError: + raise YahooError("Could not destructure response: the content contains zero-length result {}".format(content['result'])) # Note: Feel free to suggest more here via a PR. _MARKETS = { 'us_market': 'USD', 'ca_market': 'CAD', - 'ch_market': 'CHF', } @@ -85,8 +95,7 @@ def get_price_series(ticker: str, 'interval': '1d', } payload.update(_DEFAULT_PARAMS) - response = requests.get(url, params=payload, headers={'User-Agent': None}) - response.raise_for_status() + response = _requestor(url, params=payload) result = parse_response(response) meta = result['meta'] @@ -121,14 +130,8 @@ def get_latest_price(self, ticker: str) -> Optional[source.SourcePrice]: 'exchange': 'NYSE', } payload.update(_DEFAULT_PARAMS) - response = requests.get(url, params=payload, headers={'User-Agent': None}) - response.raise_for_status() - try: - result = parse_response(response) - except YahooError as error: - # The parse_response method cannot know which ticker failed, - # but the user definitely needs to know which ticker failed! - raise YahooError("%s (ticker: %s)" % (error, ticker)) from error + response = _requestor(url, params=payload) + result = parse_response(response) try: price = Decimal(result['regularMarketPrice']) From 2d4bc8308969664d726d261187e0a2440525d6c1 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 13:53:16 +0000 Subject: [PATCH 03/10] Lint fixes and remove obsolete status code check. --- beanprice/sources/yahoo.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/beanprice/sources/yahoo.py b/beanprice/sources/yahoo.py index aec9751..737004c 100644 --- a/beanprice/sources/yahoo.py +++ b/beanprice/sources/yahoo.py @@ -33,22 +33,33 @@ def _requestor(*args, **kwargs): if "headers" not in kwargs: kwargs["headers"] = {} # Yahoo! balks without this header. - kwargs["headers"]["User-Agent"] = "Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/110.0" + kwargs["headers"]["User-Agent"] = ( + "Mozilla/5.0 (X11; Linux x86_64; " + "rv:109.0) Gecko/20100101 Firefox/110.0" + ) response = requests.get(*args, **kwargs) - response.raise_for_status() + try: + response.raise_for_status() + except Exception as exc: + raise YahooError( + "HTTP status {}: {}".format( + response.status_code, + response.text(), + ) + ) from exc return response def parse_response(response: requests.models.Response) -> Dict: """Process as response from Yahoo. + Assumes the response code is among the OK response codes. + Raises: YahooError: If there is an error in the response. """ json = response.json(parse_float=Decimal) content = next(iter(json.values())) - if response.status_code != requests.codes.ok: - raise YahooError("Status {}: {}".format(response.status_code, content['error'])) if len(json) != 1: raise YahooError("Invalid format in response from Yahoo; many keys: {}".format( ','.join(json.keys()))) @@ -56,8 +67,13 @@ def parse_response(response: requests.models.Response) -> Dict: raise YahooError("Error fetching Yahoo data: {}".format(content['error'])) try: return content['result'][0] - except IndexError: - raise YahooError("Could not destructure response: the content contains zero-length result {}".format(content['result'])) + except IndexError as exc: + raise YahooError( + ( + "Could not destructure response: " + "the content contains zero-length result {}" + ).format(content['result']) + ) from exc # Note: Feel free to suggest more here via a PR. From 2db883462a929d5781c61fd449b897b2ce202100 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 13:54:38 +0000 Subject: [PATCH 04/10] Attr not func() --- beanprice/sources/yahoo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beanprice/sources/yahoo.py b/beanprice/sources/yahoo.py index 737004c..b5cd6cf 100644 --- a/beanprice/sources/yahoo.py +++ b/beanprice/sources/yahoo.py @@ -44,7 +44,7 @@ def _requestor(*args, **kwargs): raise YahooError( "HTTP status {}: {}".format( response.status_code, - response.text(), + response.text, ) ) from exc return response From ffa937c03579a9bd2f8dbfc2906ff2942902990a Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 13:55:35 +0000 Subject: [PATCH 05/10] Attribute text in mock response. --- beanprice/sources/yahoo_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beanprice/sources/yahoo_test.py b/beanprice/sources/yahoo_test.py index fc4b8b7..3bc36f8 100644 --- a/beanprice/sources/yahoo_test.py +++ b/beanprice/sources/yahoo_test.py @@ -21,6 +21,7 @@ class MockResponse: def __init__(self, contents, status_code=requests.codes.ok): self.status_code = status_code self.contents = contents + self.text = contents def json(self, **kwargs): return json.loads(self.contents, **kwargs) From eeb834877ff6234b1034db9faa0f54b339cf6ea7 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 14:01:51 +0000 Subject: [PATCH 06/10] Only catch HTTPError. --- beanprice/sources/yahoo.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beanprice/sources/yahoo.py b/beanprice/sources/yahoo.py index b5cd6cf..fb1c42a 100644 --- a/beanprice/sources/yahoo.py +++ b/beanprice/sources/yahoo.py @@ -40,11 +40,11 @@ def _requestor(*args, **kwargs): response = requests.get(*args, **kwargs) try: response.raise_for_status() - except Exception as exc: + except requests.HTTPError as exc: raise YahooError( "HTTP status {}: {}".format( response.status_code, - response.text, + response.text(), ) ) from exc return response From 69ff2081899879648e60ef9cf743fddc5e250c45 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 14:02:56 +0000 Subject: [PATCH 07/10] Add mock raise_for_status(). --- beanprice/sources/yahoo_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/beanprice/sources/yahoo_test.py b/beanprice/sources/yahoo_test.py index 3bc36f8..2ddb744 100644 --- a/beanprice/sources/yahoo_test.py +++ b/beanprice/sources/yahoo_test.py @@ -26,6 +26,10 @@ def __init__(self, contents, status_code=requests.codes.ok): def json(self, **kwargs): return json.loads(self.contents, **kwargs) + def raise_for_status(self): + if self.status_code != requests.codes.ok: + raise HTTPError(self.status_code) + class YahooFinancePriceFetcher(unittest.TestCase): From 881201d83830803bf67877a9363914c6f37ebc42 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 14:04:09 +0000 Subject: [PATCH 08/10] Correct path to class. --- beanprice/sources/yahoo_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beanprice/sources/yahoo_test.py b/beanprice/sources/yahoo_test.py index 2ddb744..7d92f09 100644 --- a/beanprice/sources/yahoo_test.py +++ b/beanprice/sources/yahoo_test.py @@ -28,7 +28,7 @@ def json(self, **kwargs): def raise_for_status(self): if self.status_code != requests.codes.ok: - raise HTTPError(self.status_code) + raise requests.HTTPError(self.status_code) class YahooFinancePriceFetcher(unittest.TestCase): From a83ddaed98ead34868448ffbff69e22b028692cf Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 14:10:04 +0000 Subject: [PATCH 09/10] Provide superior error message in case of destructuring error. --- beanprice/sources/yahoo.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/beanprice/sources/yahoo.py b/beanprice/sources/yahoo.py index fb1c42a..4cd22ae 100644 --- a/beanprice/sources/yahoo.py +++ b/beanprice/sources/yahoo.py @@ -65,15 +65,7 @@ def parse_response(response: requests.models.Response) -> Dict: ','.join(json.keys()))) if content['error'] is not None: raise YahooError("Error fetching Yahoo data: {}".format(content['error'])) - try: - return content['result'][0] - except IndexError as exc: - raise YahooError( - ( - "Could not destructure response: " - "the content contains zero-length result {}" - ).format(content['result']) - ) from exc + return content['result'][0] # Note: Feel free to suggest more here via a PR. @@ -112,7 +104,15 @@ def get_price_series(ticker: str, } payload.update(_DEFAULT_PARAMS) response = _requestor(url, params=payload) - result = parse_response(response) + try: + result = parse_response(response) + except IndexError as exc: + raise YahooError( + ( + "Could not destructure price series for ticker {}: " + "the content contains zero-length result" + ).format(ticker) + ) from exc meta = result['meta'] tzone = timezone(timedelta(hours=meta['gmtoffset'] / 3600), @@ -147,7 +147,15 @@ def get_latest_price(self, ticker: str) -> Optional[source.SourcePrice]: } payload.update(_DEFAULT_PARAMS) response = _requestor(url, params=payload) - result = parse_response(response) + try: + result = parse_response(response) + except IndexError as exc: + raise YahooError( + ( + "Could not destructure latest price for ticker {}: " + "the content contains zero-length result" + ).format(ticker) + ) from exc try: price = Decimal(result['regularMarketPrice']) From c788d131766fce6105794bfc6f5a7d340a3c3650 Mon Sep 17 00:00:00 2001 From: Rudd-O Date: Wed, 22 Feb 2023 14:12:41 +0000 Subject: [PATCH 10/10] Update test. --- beanprice/sources/yahoo_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beanprice/sources/yahoo_test.py b/beanprice/sources/yahoo_test.py index 7d92f09..57d415b 100644 --- a/beanprice/sources/yahoo_test.py +++ b/beanprice/sources/yahoo_test.py @@ -162,7 +162,9 @@ def test_parse_response_error_not_none(self): def test_parse_response_empty_result(self): response = MockResponse( '{"quoteResponse": {"error": null, "result": []}}') - with self.assertRaises(yahoo.YahooError): + with self.assertRaises(IndexError): + # Callers re-raise a YahooError from here, to provide + # superior error messages. yahoo.parse_response(response) def test_parse_response_no_timestamp(self):