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

JSON decode error fix #256

Closed

Conversation

wizedkyle
Copy link

@wizedkyle wizedkyle commented Oct 4, 2022

Outlined in #255 there can be JSON decode errors when running TheHive4Py on AWS Lambda most likely due to Lambda not including the correct package that contains JSONDecodeError.

This PR updates the JSONDecodeError to come from the requests package. Thank you to all the hard work from @octocolby for identifying the root cause of this issue and providing a fix.

@Kamforka
Copy link
Collaborator

Kamforka commented Oct 4, 2022

I've made a fix for this one before, but it seems like the PR got lost in the meantime.

Did you try this fix against simplejson?

In case it doesn't work with other libs please fall back using this snippet:

import json as jsonlib

And inside the try block use the standard lib to decode JSON instead of the builtin json() method of requests.

json_data = jsonlib.loads(response.content)

@Kamforka Kamforka closed this Oct 4, 2022
@Kamforka Kamforka reopened this Oct 4, 2022
@wizedkyle
Copy link
Author

@Kamforka No we didn't try against simplejson however, requests will decide if there is simplejson or json so wouldn't that be ideal as you don't have to worry about the lib it will determine it for you?

I will add my Python knowledge isn't as good as the people here so i will also defer to @octocolby if he has any other thoughts.

https://github.com/psf/requests/blob/bda7f0171f8bba17989d3a2c28dfa9a9261b1b65/requests/compat.py#L39

@Kamforka
Copy link
Collaborator

Kamforka commented Oct 4, 2022

@wizedkyle I'm not sure requests will choose the right decode error as there is a thread around this issue for years now: psf/requests#4842

I don't mind your current solution if you can verify it against simplejson, I'd do that myself but for the time being I don't have access to my workspace, I can check it earliest end of this week.

@Kamforka Kamforka added this to the 2.0.0b3 milestone Oct 4, 2022
@Kamforka Kamforka linked an issue Oct 4, 2022 that may be closed by this pull request
@wizedkyle
Copy link
Author

@Kamforka I am going to test the fix against simplejson tonight and will let you know the results.

@wizedkyle
Copy link
Author

@Kamforka using simplejson fixes the issue as well, so which fix do you want to use?

@Kamforka
Copy link
Collaborator

@wizedkyle Maybe I explained myself poorly. I didn't mean to use the simplejson inside thehive4py, what I meant is that your original fix should be tested against an environment where simplejson is present, i.e. pip install simplejson thehive4py in your testing environment.

But no worries now I also have to verify it on my own, I'll get back to you.

@Kamforka
Copy link
Collaborator

Okay I've checked the requests way of doing things and I'm not sure why it worked for you but I immediately get this error:

>>> from requests import JSONDecodeError
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'JSONDecodeError' from 'requests'

So I'd just suggest to fall back to my original recommendation and use the standard json library to parse the content of the response:

json_data = jsonlib.loads(response.content)

May I ask you to add this change to the PR? You need to adjust _process_text_response and _process_error_response for this.

I also tested it in an environment with and without simplejson and it worked fine.

@Kamforka
Copy link
Collaborator

Fixing this in #259

@Kamforka Kamforka closed this Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSONDecodeError not caught correctly when using AWS Lambda
2 participants