-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
Django backend fails on exception with non-serializable argument #46
Comments
Dear @cjerdonek I have some concerns about that use case. As you mentioned, application might raise an exception, which is not under developer's control. If library would not validate the type, it might affect frontend developers later on. My suggestion would be to inherit from current serializer and inject it to jsonrpc manager. from jsonrpc.utils import DatetimeDecimalEncoder
class MyEncoder(DatetimeDecimalEncoder):
def default(self, o):
try:
return super(MyEncoder, self).default(self, o)
except TypeError:
return repr(o) Then this encoder could be used here: https://github.com/pavlov99/json-rpc/blob/master/jsonrpc/backend/django.py#L62 Please let me know if that works. |
@pavlov99 Thanks for such a prompt reply. It seems to me though that json-rpc has a responsibility to meet its guarantee and return valid JSON. Every application is susceptible to this issue. (Incidentally, it also seems that the issue isn't limited to the Django backend as you will see below.) In this function, |
Another suggestion: if you are concerned about backwards-forwards compatibility, etc, how about handling the serialization exception and raising a serializable exception saying something like, "unserializable error raised with type: ..."? That way developers will at least be alerted to the issue on a case-by-case basis, but as JSON as opposed to 500 response. |
For the record, I just want to include this from the JSON-RPC 2.0 Specification, section 5 Response object:
Thus, this issue is about a case in which json-rpc does not respond with a JSON object. Json-rpc should have some kind of global exception handler to ensure that it always returns a JSON response, regardless of the underlying application code. |
@cjerdonek absolutely agree with the latest point. Let me add tests for Django 1.9. |
I am facing the same problem. Any news on this? |
I encountered a situation where json-rpc will break on certain exceptions. This is with json-rpc version 1.10.3 and Django 1.9.6.
If an exception is raised with code like
raise Exception(foo)
, wherefoo
is not serializable, then json-rpc doesn't return a proper JSON response.Here is an example stack trace:
In the case of handling an exception, it seems like json-rpc should be able to fall back to something like calling
repr()
on the exception, instead of insisting that the exception be serializable.This is because the developer doesn't necessarily have control over what exceptions are raised (e.g. if they come from third-party code, etc).
The text was updated successfully, but these errors were encountered: