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

Django backend fails on exception with non-serializable argument #46

Open
cjerdonek opened this issue May 14, 2016 · 6 comments
Open

Django backend fails on exception with non-serializable argument #46

cjerdonek opened this issue May 14, 2016 · 6 comments

Comments

@cjerdonek
Copy link
Contributor

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), where foo is not serializable, then json-rpc doesn't return a proper JSON response.

Here is an example stack trace:

Traceback (most recent call last):
  File ".../python3.5/site-packages/django/core/handlers/base.py", line 149, in get_response
    response = self.process_exception_by_middleware(e, request)
  File ".../python3.5/site-packages/django/core/handlers/base.py", line 147, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File ".../python3.5/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File ".../python3.5/site-packages/jsonrpc/backend/django.py", line 65, in jsonrpc
    response = response.json
  File ".../python3.5/site-packages/jsonrpc/base.py", line 85, in json
    return self.serialize(self.data)
  File ".../python3.5/site-packages/jsonrpc/backend/django.py", line 62, in serialize
    return json.dumps(s, cls=DatetimeDecimalEncoder)
  File "/usr/lib/python3.5/json/__init__.py", line 237, in dumps
    **kw).encode(obj)
  File "/usr/lib/python3.5/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib/python3.5/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File ".../python3.5/site-packages/jsonrpc/utils.py", line 53, in default
    return json.JSONEncoder.default(self, o)
  File "/usr/lib/python3.5/json/encoder.py", line 180, in default
    raise TypeError(repr(o) + " is not JSON serializable")
TypeError: <Foo object> is not JSON serializable

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).

@pavlov99
Copy link
Owner

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.

@cjerdonek
Copy link
Contributor Author

@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, JSONRPCResponseManager constructs a JSONRPCServerError when an exception occurs. However, it makes the assumption that e.args is JSON serializable, which isn't necessarily the case. It seems json-rpc shouldn't be creating a JSONRPCError object there if it can't guarantee that the object is actually serializable.

@cjerdonek
Copy link
Contributor Author

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.

@cjerdonek
Copy link
Contributor Author

For the record, I just want to include this from the JSON-RPC 2.0 Specification, section 5 Response object:

When a rpc call is made, the Server MUST reply with a Response, except for in the case of Notifications. The Response is expressed as a single JSON Object, with the following members:

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.

@pavlov99
Copy link
Owner

@cjerdonek absolutely agree with the latest point. Let me add tests for Django 1.9.

@pavlov99 pavlov99 self-assigned this May 31, 2016
@arnuschky
Copy link
Contributor

I am facing the same problem. Any news on this?

arnuschky added a commit to arnuschky/json-rpc that referenced this issue Aug 25, 2016
arnuschky added a commit to arnuschky/json-rpc that referenced this issue Aug 25, 2016
arnuschky added a commit to arnuschky/json-rpc that referenced this issue Aug 25, 2016
arnuschky added a commit to arnuschky/json-rpc that referenced this issue Aug 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants