razorpay / razorpay-python

Razorpay Python SDK
https://pypi.python.org/pypi/razorpay
MIT License
153 stars 81 forks source link

Fix: Added decimal encoder #227

Closed amaan-ahmad closed 1 year ago

amaan-ahmad commented 2 years ago

Explanation of the Issue I had given a fix for.

Hi team,

There was an unhandled exception on the client SDK, that if a Decimal type is passed as an amount while creating order it raised the following error, TypeError: Object of type Decimal is not JSON serializable

File "/home/ubuntu/.local/share/virtualenvs/tortoise-customer-webserver-x9341mt2/lib/python3.7/site-packages/razorpay/client.py", line 183, in _update_request
    data = json.dumps(data)
  File "/home/ubuntu/.pyenv/versions/3.7.13/lib/python3.7/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
  File "/home/ubuntu/.pyenv/versions/3.7.13/lib/python3.7/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/home/ubuntu/.pyenv/versions/3.7.13/lib/python3.7/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/home/ubuntu/.pyenv/versions/3.7.13/lib/python3.7/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '

This will be common for Django environment, as I had previously came across the same issue with some other package, thereby contributing a fix. However, the Razorpay backend would anyway return a 400 error stating that the amount should be int.

amaan-ahmad commented 1 year ago

Hi maintainers, Can you please review this PR?

@sidag95 @ankitdas13

ankitdas13 commented 1 year ago

Hi @amaan-ahmad i will check this code and let you know .

ankitdas13 commented 1 year ago

@amaan-ahmad could you please let me know why you are converting the amount into string? Because according to doc the amount should be integer. It does not support decimal.

ankitdas13 commented 1 year ago

@amaan-ahmad could you please respond?

amaan-ahmad commented 1 year ago

Hi @ankitdas13 , apologies for the late response. yes the amount should be int but my only point is that SDK itself raises exception due to JSON parse error, which shouldn't happen. Instead the razorpay backend shall return error response.

ankitdas13 commented 1 year ago

Hi @amaan-ahmad i got the point, but till now most of the users are sending the amount without Decimal, like we mentioned in Doc, and then they will get razorpay backend error response if the amount value is in decimal form . We haven't seen any users experience this error TypeError: Object of type Decimal is not JSON serializable. Even I didn't face this issue until i use Decimal.

Screenshot 2022-11-07 at 1 51 33 PM
ankitdas13 commented 1 year ago

@amaan-ahmad Please let me know if you have any doubts

amaan-ahmad commented 1 year ago

got you. But I had suggest that support for decimal might be a helpful thing. But in case, if we decide not to support decimal then we can discard this. Thanks for your time :)

ankitdas13 commented 1 year ago

@amaan-ahmad It would be greatly appreciated if you kept contributing in this SDK. Closing this issue for now.