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

Invalid JSON parsing datetime items #411

Closed
solracfs opened this issue Jun 4, 2020 · 6 comments
Closed

Invalid JSON parsing datetime items #411

solracfs opened this issue Jun 4, 2020 · 6 comments

Comments

@solracfs
Copy link

solracfs commented Jun 4, 2020

Description of the issue:
An invalid JSON is generated with a dictionary that has a datetime item, instead of throwing an exception

Expected result:
TypeError: datetime.datetime(...) is not JSON serializable

Actual result:
The function returns an invalid encoded JSON with empty value:
{"d":,"n":null}

Versions:

  • OS: Ubuntu 19.10
  • Python: 3.7.5
  • UltraJSON: 3.0.0

Code to reproduce the issue:

import ujson
from datetime import datetime

print(ujson.dumps({ "d": datetime.now(), "n": None }))
@shinybrar
Copy link

shinybrar commented Jun 5, 2020

I can confirm the issue has been around since release version 2.0.1, where as 1.35 works as expected.

3.0

import ujson
from datetime import datetime
print(ujson.dumps({ "d": datetime.now(), "n": None }))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: datetime.datetime(2020, 6, 5, 16, 8, 50, 798045) is not JSON serializable

2.0.1

import ujson
from datetime import datetime
print(ujson.dumps({ "d": datetime.now(), "n": None }))

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: � is not JSON serializable

1.35

 import ujson
from datetime import datetime
print(ujson.dumps({ "d": datetime.now(), "n": None }))
{"d":1591372908,"n":null}

Edit: Can also confirm, none of the releases between 2.0.1 and 3.0 seem to work.

@solracfs
Copy link
Author

solracfs commented Jun 8, 2020

Thanks for checking, @shinybrar.

According to your output, it seems that the test code is throwing a TypeError exception in your tests.
This is the expected result, since support for datetime objects was removed in version 2.
However, this is not the same result as mine. Note the empty value for "d":

{"d":,"n":null}

To avoid differences in OS, python versions, etc., I have tested the code in a docker container:

Dockerfile:

FROM python:3.8.3

WORKDIR /usr/src/app

RUN pip install ujson

CMD ["python", "-c", "import ujson;from datetime import datetime;print(ujson.dumps({ 'd': datetime.now(), 'n': None }))"]
$ docker build -t test-ujson-app .
$ docker run -it --rm --name test-ujson-app test-ujson-app

Output:

{"d":,"n":null}

NOTE: if you remove the None element, leaving only the datetime value, it works as expected, throwing a TypeError exception.

@shinybrar
Copy link

@solracfs Thanks for the clarification. I think the exception due to deprecation makes sense.

@champax
Copy link

champax commented Feb 2, 2021

Though i understand that json standard do not defines date serialization and that support removal is compliant with this, ujson do not support for dumps(d, cls=MyEncoder) as standard json lib support.
We are using ujson for performance reason, i will not patch bunches of client code neither massive json input coming from external stuff to handle custom pre-sererialization before calling ujson.dumps

At this stage, i have to revert to json...

I saw bunches of issues related to this into ujson, but the support from "cls=" was never put in place. Do we have a chance to have this one day?

@JustAnotherArchivist
Copy link
Collaborator

I can no longer reproduce this with ujson 5.2.0. I think it was probably fixed by #505.

@hugovk
Copy link
Member

hugovk commented Apr 7, 2022

Thanks, let's close!

@hugovk hugovk closed this as completed Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants