json.loads() should return a more specific error

[Roy Smith]

Before I go open an enhancement request, what do people think of the
idea that json.load() should return something more specific than
ValueError?

I've got some code that looks like

try:
response = requests.get(url)
except RequestException as ex:
logger.exception(ex)
return []
data = response.text
try:
events = json.loads(data)
except ValueError as ex:
logger.error("%s: %r", ex, data)
return []

This would be so much neater if json would return something I could
identify as a json error. It would all just collapse into:

try:
events = requests.get(url).json
except (RequestException, JSONDecodeError) as ex:
logger.exception(ex)
return []

We could make JSONDecodeError a subclass of ValueError so existing code
would continue to work.

 

[Terry Reedy]

Before I go open an enhancement request, what do people think of the
idea that json.load() should return something more specific than
ValueError?

I do not know of any written policy about when to create custom error
classes in the stdlib. I know there are some that are just empty
catchall renamings.

"class ModException(Exception): pass"

This does not seem to have much use except your use us scanning logs,
and does have some cost.

I've got some code that looks like

try:
response = requests.get(url)
except RequestException as ex:
logger.exception(ex)
return []
data = response.text
try:
events = json.loads(data)
except ValueError as ex:
logger.error("%s: %r", ex, data)
return []

You could solve your immediate problem by starting the string with
something like 'JSON: '. One might want the url included in the log,
which would never be part of an exception from json. Given that data can
be arbitrarily long, logging data does not seem like that good an idea.

To me, the important question is not the exception name, which you can
replace, but whether the message part of the exception gives information
about the actual problem. If it just says something redundant and
useless like 'bad input', then improving that would be a good enhancement.

This would be so much neater if json would return something I could
identify as a json error. It would all just collapse into:

try:
events = requests.get(url).json

Would not this be
events = json.loads(requests.get(url).text)
?
Either way, 'events' is the only new binding that persists.

except (RequestException, JSONDecodeError) as ex:

Just using ValueError would work if you condition the logging on the
value of ex.

logger.exception(ex)

This would only be the equivalent of your first code if the arbitrarily
large input data were attached to the JSONDecodeError -- and thereby
kept alive when it could otherwise be deleted/collected (and the custom
class had its own __str__ method). I do not think this a good idea. The
exception should only contain extracted bits that show the problem.

return []

We could make JSONDecodeError a subclass of ValueError so existing code
would continue to work.

Bottom line: I do not think you should expect exception instances to
necessarily have all the info you would want logged for reading out of
context (as opposed to reading interactively). On the other hand,
exceptions should contain specific information about the problem that
the raising code knows and that is hard to get otherwise.