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

Fix inconsistent exception type in response.json() method #5856

Merged
merged 28 commits into from Jul 26, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5ff9fb7
Set up json fix option 2
steveberdy Jul 2, 2021
d66d842
Fix incorrect comment in models.py
steveberdy Jul 2, 2021
a072b5e
Added support to consistently raise JSONDecodeError and ValuError, no…
steveberdy Jul 3, 2021
fc1a70f
Reduce confusion in error import naming
steveberdy Jul 3, 2021
1ac7bfe
Minor naming changes
steveberdy Jul 3, 2021
0045b83
Added check for Python 2 vs. Python 3 when importing json.JSONDecodeE…
steveberdy Jul 3, 2021
482578e
Update compat.py
steveberdy Jul 5, 2021
ac07d45
Update models.py
steveberdy Jul 5, 2021
91d9bc8
Update models.py
steveberdy Jul 5, 2021
ee45190
Update models.py
steveberdy Jul 5, 2021
66fc86e
Update models.py
steveberdy Jul 5, 2021
15bd4c5
Update quickstart.rst
steveberdy Jul 5, 2021
6705154
Update HISTORY.md
steveberdy Jul 5, 2021
9f0cbdd
Merge branch 'master' into json-opt-2
steveberdy Jul 7, 2021
12a55d8
Delete test_json.py
steveberdy Jul 7, 2021
a93d4af
Replaced json with complexjson
steveberdy Jul 8, 2021
5193ec5
Edited documentation and other minor changes
steveberdy Jul 9, 2021
3230e23
Made JSONDecodeError a subclass of RequestException and changed docs …
steveberdy Jul 9, 2021
f76f6d1
Attempted fix of method resolution order error
steveberdy Jul 12, 2021
1d6d963
Improved backwards compatibility for exceptions
steveberdy Jul 12, 2021
5c6be66
Update quickstart.rst
steveberdy Jul 12, 2021
b6b94e1
Raise JSONDecodeError without args when in Python 2 env
steveberdy Jul 12, 2021
090fe0e
Edited old GitHub API endpoint in api.rst
steveberdy Jul 12, 2021
2741955
Merge branch 'master' into json-opt-2
steveberdy Jul 13, 2021
e18e879
Raise error with message in Python 2
steveberdy Jul 14, 2021
bcfde77
Merge branch 'json-opt-2' of https://github.com/steveberdy/requests i…
steveberdy Jul 14, 2021
64b3f18
Push docs to next release
steveberdy Jul 15, 2021
8f0dcc4
Cleared confusion in exceptions.py. Added test
steveberdy Jul 23, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions docs/user/quickstart.rst
Expand Up @@ -150,9 +150,7 @@ There's also a builtin JSON decoder, in case you're dealing with JSON data::

In case the JSON decoding fails, ``r.json()`` raises an exception. For example, if
the response gets a 204 (No Content), or if the response contains invalid JSON,
attempting ``r.json()`` raises ``simplejson.JSONDecodeError`` if simplejson is
installed or raises ``ValueError: No JSON object could be decoded`` on Python 2 or
``json.JSONDecodeError`` on Python 3.
attempting ``r.json()`` raises ``requests.JSONDecodeError``.
steveberdy marked this conversation as resolved.
Show resolved Hide resolved

It should be noted that the success of the call to ``r.json()`` does **not**
indicate the success of the response. Some servers may return a JSON object in a
Expand Down
2 changes: 1 addition & 1 deletion requests/__init__.py
Expand Up @@ -124,7 +124,7 @@ def _check_cryptography(cryptography_version):
from .exceptions import (
RequestException, Timeout, URLRequired,
TooManyRedirects, HTTPError, ConnectionError,
FileModeWarning, ConnectTimeout, ReadTimeout
FileModeWarning, ConnectTimeout, ReadTimeout, JSONDecodeError
)

# Set default logging handler to avoid "No handler found" warnings.
Expand Down
6 changes: 1 addition & 5 deletions requests/compat.py
Expand Up @@ -8,6 +8,7 @@
Python 3.
"""

import json
import chardet

import sys
Expand All @@ -25,10 +26,6 @@
#: Python 3.x?
is_py3 = (_ver[0] == 3)

try:
import simplejson as json
except ImportError:
import json
steveberdy marked this conversation as resolved.
Show resolved Hide resolved

steveberdy marked this conversation as resolved.
Show resolved Hide resolved
# ---------
# Specifics
Expand All @@ -46,7 +43,6 @@
# Keep OrderedDict for backwards compatibility.
from collections import Callable, Mapping, MutableMapping, OrderedDict


builtin_str = str
bytes = str
str = unicode
Expand Down
14 changes: 14 additions & 0 deletions requests/exceptions.py
Expand Up @@ -8,6 +8,16 @@
"""
from urllib3.exceptions import HTTPError as BaseHTTPError

try:
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
from json import JSONDecodeError as StandardJSONDecodeError
except ImportError: # Python 2, ValueError is raised
StandardJSONDecodeError = ValueError

try:
from simplejson import JSONDecodeError as SimpleJSONDecodeError
except ImportError:
SimpleJSONDecodeError = Exception


class RequestException(IOError):
"""There was an ambiguous exception that occurred while handling your
Expand All @@ -25,6 +35,10 @@ def __init__(self, *args, **kwargs):
super(RequestException, self).__init__(*args, **kwargs)


class JSONDecodeError(StandardJSONDecodeError, SimpleJSONDecodeError):
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
"""Couldn't decode the text into json"""


class InvalidJSONError(RequestException):
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
"""A JSON error occurred."""

Expand Down
62 changes: 37 additions & 25 deletions requests/models.py
Expand Up @@ -29,7 +29,8 @@
from .cookies import cookiejar_from_dict, get_cookie_header, _copy_cookie_jar
from .exceptions import (
HTTPError, MissingSchema, InvalidURL, ChunkedEncodingError,
ContentDecodingError, ConnectionError, StreamConsumedError, InvalidJSONError)
ContentDecodingError, ConnectionError, StreamConsumedError,
InvalidJSONError, JSONDecodeError)
from ._internal_utils import to_native_string, unicode_is_ascii
from .utils import (
guess_filename, get_auth_from_url, requote_uri,
Expand All @@ -39,7 +40,7 @@
Callable, Mapping,
cookielib, urlunparse, urlsplit, urlencode, str, bytes,
is_py2, chardet, builtin_str, basestring)
from .compat import json as complexjson
from .compat import json
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
from .status_codes import codes

#: The set of HTTP status codes that indicate an automatically
Expand Down Expand Up @@ -176,12 +177,14 @@ def register_hook(self, event, hook):
"""Properly register a hook."""

if event not in self.hooks:
raise ValueError('Unsupported event specified, with event name "%s"' % (event))
raise ValueError(
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
'Unsupported event specified, with event name "%s"' % (event))

if isinstance(hook, Callable):
self.hooks[event].append(hook)
elif hasattr(hook, '__iter__'):
self.hooks[event].extend(h for h in hook if isinstance(h, Callable))
self.hooks[event].extend(
h for h in hook if isinstance(h, Callable))
steveberdy marked this conversation as resolved.
Show resolved Hide resolved

def deregister_hook(self, event, hook):
"""Deregister a previously registered hook.
Expand Down Expand Up @@ -224,8 +227,8 @@ class Request(RequestHooksMixin):
"""

def __init__(self,
method=None, url=None, headers=None, files=None, data=None,
params=None, auth=None, cookies=None, hooks=None, json=None):
method=None, url=None, headers=None, files=None, data=None,
params=None, auth=None, cookies=None, hooks=None, json=None):
steveberdy marked this conversation as resolved.
Show resolved Hide resolved

# Default empty dicts for dict params.
data = [] if data is None else data
Expand Down Expand Up @@ -308,8 +311,8 @@ def __init__(self):
self._body_position = None

def prepare(self,
method=None, url=None, headers=None, files=None, data=None,
params=None, auth=None, cookies=None, hooks=None, json=None):
method=None, url=None, headers=None, files=None, data=None,
params=None, auth=None, cookies=None, hooks=None, json=None):
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
"""Prepares the entire request with the given parameters."""

self.prepare_method(method)
Expand Down Expand Up @@ -384,7 +387,8 @@ def prepare_url(self, url, params):
raise InvalidURL(*e.args)

if not scheme:
error = ("Invalid URL {0!r}: No schema supplied. Perhaps you meant http://{0}?")
error = (
"Invalid URL {0!r}: No schema supplied. Perhaps you meant http://{0}?")
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
error = error.format(to_native_string(url, 'utf8'))

raise MissingSchema(error)
Expand Down Expand Up @@ -438,7 +442,8 @@ def prepare_url(self, url, params):
else:
query = enc_params

url = requote_uri(urlunparse([scheme, netloc, path, None, query, fragment]))
url = requote_uri(urlunparse(
[scheme, netloc, path, None, query, fragment]))
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
self.url = url

def prepare_headers(self, headers):
Expand Down Expand Up @@ -468,9 +473,9 @@ def prepare_body(self, data, files, json=None):
content_type = 'application/json'

try:
body = complexjson.dumps(json, allow_nan=False)
body = json.dumps(json, allow_nan=False)
except ValueError as ve:
raise InvalidJSONError(ve, request=self)
raise InvalidJSONError(ve, request=self)

if not isinstance(body, bytes):
body = body.encode('utf-8')
Expand Down Expand Up @@ -500,7 +505,8 @@ def prepare_body(self, data, files, json=None):
self._body_position = object()

if files:
raise NotImplementedError('Streamed bodies and files are mutually exclusive.')
raise NotImplementedError(
'Streamed bodies and files are mutually exclusive.')
steveberdy marked this conversation as resolved.
Show resolved Hide resolved

if length:
self.headers['Content-Length'] = builtin_str(length)
Expand Down Expand Up @@ -776,7 +782,8 @@ def generate():
if self._content_consumed and isinstance(self._content, bool):
raise StreamConsumedError()
elif chunk_size is not None and not isinstance(chunk_size, int):
raise TypeError("chunk_size must be an int, it is instead a %s." % type(chunk_size))
raise TypeError(
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
"chunk_size must be an int, it is instead a %s." % type(chunk_size))
# simulate reading small chunks of the content
reused_chunks = iter_slices(self._content, chunk_size)

Expand Down Expand Up @@ -833,7 +840,8 @@ def content(self):
if self.status_code == 0 or self.raw is None:
self._content = None
else:
self._content = b''.join(self.iter_content(CONTENT_CHUNK_SIZE)) or b''
self._content = b''.join(
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
self.iter_content(CONTENT_CHUNK_SIZE)) or b''

self._content_consumed = True
# don't need to release the connection; that's been handled by urllib3
Expand Down Expand Up @@ -882,12 +890,8 @@ def json(self, **kwargs):
r"""Returns the json-encoded content of a response, if any.

:param \*\*kwargs: Optional arguments that ``json.loads`` takes.
:raises simplejson.JSONDecodeError: If the response body does not
contain valid json and simplejson is installed.
:raises json.JSONDecodeError: If the response body does not contain
valid json and simplejson is not installed on Python 3.
:raises ValueError: If the response body does not contain valid
json and simplejson is not installed on Python 2.
:raises requests.JSONDecodeError: If the response body does not
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
contain valid json.
"""

if not self.encoding and self.content and len(self.content) > 3:
Expand All @@ -898,7 +902,7 @@ def json(self, **kwargs):
encoding = guess_json_utf(self.content)
if encoding is not None:
try:
return complexjson.loads(
return json.loads(
self.content.decode(encoding), **kwargs
)
except UnicodeDecodeError:
Expand All @@ -907,7 +911,13 @@ def json(self, **kwargs):
# and the server didn't bother to tell us what codec *was*
# used.
pass
return complexjson.loads(self.text, **kwargs)

try:
return json.loads(self.text, **kwargs)
except json.JSONDecodeError as e:
# Catch JSON-related errors and raise as requests.JSONDecodeError
# This aliases json.JSONDecodeError and simplejson.JSONDecodeError
raise JSONDecodeError(e.msg, e.doc, e.pos)
steveberdy marked this conversation as resolved.
Show resolved Hide resolved

@property
def links(self):
Expand Down Expand Up @@ -944,10 +954,12 @@ def raise_for_status(self):
reason = self.reason

if 400 <= self.status_code < 500:
http_error_msg = u'%s Client Error: %s for url: %s' % (self.status_code, reason, self.url)
http_error_msg = u'%s Client Error: %s for url: %s' % (
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
self.status_code, reason, self.url)

elif 500 <= self.status_code < 600:
http_error_msg = u'%s Server Error: %s for url: %s' % (self.status_code, reason, self.url)
http_error_msg = u'%s Server Error: %s for url: %s' % (
steveberdy marked this conversation as resolved.
Show resolved Hide resolved
self.status_code, reason, self.url)

if http_error_msg:
raise HTTPError(http_error_msg, response=self)
Expand Down
27 changes: 27 additions & 0 deletions tests/test_json.py
@@ -0,0 +1,27 @@
import pytest
import simplejson
import json

from requests import get, JSONDecodeError

success_url = "https://httpbin.org/get" # returns JSON
failure_url = "https://google.com" # doesn't return JSON


def test_json_decode_success():
assert isinstance(get(success_url).json(), dict)


def test_json_decode_failure_catch():
# test that all exceptions can be caught
with pytest.raises(json.JSONDecodeError):
get(failure_url).json()

with pytest.raises(simplejson.JSONDecodeError):
get(failure_url).json()

with pytest.raises(JSONDecodeError):
get(failure_url).json()

with pytest.raises(ValueError):
get(failure_url).json()