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

Unexpected Behavior: 500 Internal Server Error unhandled by PyCap #261

Open
angus-lherrou opened this issue Apr 13, 2023 · 4 comments · May be fixed by #263
Open

Unexpected Behavior: 500 Internal Server Error unhandled by PyCap #261

angus-lherrou opened this issue Apr 13, 2023 · 4 comments · May be fixed by #263

Comments

@angus-lherrou
Copy link

Describe the behavior:

When a request is made with PyCap that results in a 500 Internal Server Error response, PyCap's _RCRequest.get_content does not handle this error; instead, with format_type="csv"|"xml", an empty string from response.text is silently returned, and with format_type="json", a requests.exceptions.JSONDecodeError('Expecting value: line 1 column 1 (char 0)') is raised due to response.text being an empty string.

Expected behavior:

The expected behavior would be to detect a 500 code in the request response and emit an informative error from get_content.

Desktop:

  • OS: Ubuntu 20.04.5 LTS x86_64
  • REDCap version 12.4.7
  • PyCap Version 2.4.0
@pwildenhain
Copy link
Collaborator

Thanks for raising the issue -- what is the body of the response that PyCap fails to capture -- i.e what error message shows for a 500 response?

@angus-lherrou
Copy link
Author

@pwildenhain response.text is an empty string and response.reason is 'Internal Server Error'.

@pwildenhain
Copy link
Collaborator

pwildenhain commented Apr 15, 2023

Got it!

Looks like this piece of the code needs to be updated:

PyCap/redcap/request.py

Lines 196 to 209 in a507d1a

if self.fmt == "json":
try:
bad_request = "error" in content.keys() # type: ignore
except AttributeError:
# we're not dealing with an error dict
bad_request = False
elif self.fmt == "csv":
bad_request = content.lower().startswith("error:") # type: ignore
# xml is the default returnFormat for error messages
elif self.fmt == "xml" or self.fmt is None:
bad_request = "<error>" in str(content).lower()
if bad_request:
raise RedcapError(content)

We should check the status code to see if it's 500 (or one of it's variants?), and if so, then raise RedcapError(response.reason). We can probably add this check before/after the if block that checks the content for a bad_request

Would you be open to making this contribution? See here for what that might entail: https://github.com/redcap-tools/PyCap/blob/master/CONTRIBUTING.md

@angus-lherrou
Copy link
Author

Happy to open a PR.

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

Successfully merging a pull request may close this issue.

2 participants