yoshida-lab/XenonPy

HTTPError message does not shown when response return 504

Closed this issue · 6 comments

Yamazaki-Youichi commented

Hello. Thank you for the awesome package.
I have founded trivial things regarding the raise exception.

  1. I would like to check which models are prepared. Could you provide me the list of trained models, for my research purpose?
  2. In addition, I have a simple query to get all Models data.
    When server return the 504 status, the raise message did not match as your expectation.
mdl = MDL()
models = mdl("_", save_to=False)
C:\Anaconda3\lib\json\decoder.py in raw_decode(self, s, idx)
    355             obj, end = self.scan_once(s, idx)
    356         except StopIteration as err:
--> 357             raise JSONDecodeError("Expecting value", s, err.value) from None
    358         return obj, end

JSONDecodeError: Expecting value: line 1 column 1 (char 0)

When 504 error, ret.json() doesn't success because the return value is not json, since
JSONDecodeError overwrite raise HTTPError.

    def query(self, query, variables):
        payload = json.dumps({'query': query, 'variables': variables})
        ret = requests.post(url=self._url, headers=self._headers, data=payload)
        if ret.status_code != 200:
            raise HTTPError('status_code: %s, %s' %
                            (ret.status_code, ret.json()))
        ret = ret.json()['data']
        return ret
ipdb> ret
<Response [504]>
ipdb> ret.content
b'<html>\r\n<head><title>504 Gateway Time-out</title></head>\r\n<body bgcolor="white">\r\n<center><h1>504 Gateway Time-out</h1></center>\r\n<hr><center>nginx/1.14.2</center>\r\n</body>\r\n</html>\r\n'
ipdb> ret.json()
*** json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

I'm looking forward for your kind reply. Thank you very much.

TsumiNa commented

Hi @Yamazaki-Youichi, Thank you for your feedback!

If you want to list all these model informations. Use the method like this:

mdl = MDL()
results = mdl("", save_to=False)

The _ in python mean ignore some var but when use it a string that's not make sense.

for 1). Yes, as you said we need to publish our model list and also some additional informations as soon as possible. We wish that it can be done in next week.

for 2). I think the query string _ lead to this error because all modelsets' name contain character _. This lead to a very long time querying and a huge size response body.
We are planning to add some simple query statements e.g listModeSets, listProperties and bring them to users in version 0.3.0.

TsumiNa commented

@Yamazaki-Youichi, I have created a new issue #65 to collect the suggestion of high priority API.

Yamazaki-Youichi commented

Thank you for your reply and created new issue!
I will wait for the new version. I'm sorry to trouble you.

Regarding to the JSONDecodeError raise issue,
in my opinion, the raising of JSONDecodeError to overwrite HTTPError is misleading.
For example, when server cause error by such bad query "_", it always needs to raise HTTPError.
However, in the present code, since JSONDecodeError is raised, there is no indication of bad queries.
I'm not good at coding but here is my suggestions:

    def query(self, query, variables):
        payload = json.dumps({'query': query, 'variables': variables})
        ret = requests.post(url=self._url, headers=self._headers, data=payload)
        if ret.status_code != 200:
            # Here, When 504 error, ret.json() will raise error.
            try:
                message = ret.json()
            except JSONDecodeError:
                message = "Server does not return value."
            raise HTTPError('status_code: %s, %s' %
                            (ret.status_code, message))
        ret = ret.json()['data']
        return ret

If it's not too much trouble, I will send pull request.

  • 👍1
TsumiNa commented

@Yamazaki-Youichi Yes, we should raise an HTTPError to give clear information to users. Thanks for your pr!

Yamazaki-Youichi commented

Thank you! I sent PR at
#66

  • 🎉1