Decoding exceptions thrown by ujson are not drop-in compatible

[JustAnotherArchivist]

What did you do?

try:
	json.loads('{')
except json.JSONDecodeError as e:
	print(e)

What did you expect to happen?

The error gets caught and printed. This is the behaviour when using import json. Although I haven't tested them, it appears that other drop-in replacements like hyperjson and orjson should also behave like this.

What actually happened?

With import ujson as json, a ValueError exception followed by an AttributeError because module 'ujson' has no attribute 'JSONDecodeError'.

What versions are you using?

• OS: Debian Sid
• Python: 3.8
• UltraJSON: 5.1.0

Solution

ujson should either effectively from json import JSONDecodeError (hyperjson does this) or define its own JSONDecodeError (orjson does this) and then raise that instead of a ValueError. As JSONDecodeError is a subclass of ValueError, this would not break existing code using except ValueError.

[bwoodsend]

Sounds reasonable. I think I'd prefer the second option - to have our own JSONDecodeError rather than pulling it from json.

[hugovk]

PR welcome!

[JustAnotherArchivist]

I'm giving that a shot. Any advice on where to put the JSONDecodeError variable? It needs to be available from both ujson.c and JSONtoObj.c, of course...

[bwoodsend]

I'd have thought the header file lib/ultrajson.h. You'll probably need to declare it only in the header file with extern then initialise it in ujson.c but I'm just guessing.

[JustAnotherArchivist]

That'd work, although having a PyObject* in lib/ rather than python/ doesn't sound right (even if it's just the declaration). A separate header file for just this one variable also seems awkward though.

[bwoodsend]

Hmm, you're right. I believe that the intention is to keep the Python binding separate from the json reading/writing. Honestly, the separate header sounds best.

[hugovk]

orjson also subclasses JSONEncodeError from TypeError.

Is it worth doing the same?

(If so, could be done after #498.)

[JustAnotherArchivist]

Perhaps, although at least that's not a compatibility issue since the built-in json doesn't have it. Might be nice to have a more specific exception though.

[JustAnotherArchivist]

@hugovk Regarding JSONEncodeError, it's a bit different due to the default callable. Such a function should raise a TypeError when it can't handle a variable. But then encoding an object might raise either of those two exceptions, so you'd need to use except TypeError to catch them anyway. In other words, the value of a separate JSONEncodeError seems rather limited to me.

(By the way, confusingly and contradicting the code I linked above, orjson.JSONEncodeError is actually an alias for TypeError, not a real subclass, although a type is technically considered a subclass of itself.)

[hugovk]

Thanks, we can skip JSONEncodeError.