fix(response): reoder exception handling to correctly parse a broken response

SlowMo24 commented 2 months ago

In general, it would be nice to do only one thing in the try-block.

Then it is easier to parse the exceptions. What can go wrong where.

You suggest placing line 341 https://github.com/GIScience/ohsome-py/pull/164/files#diff-0684a040fd4741e6267194be04e688f118c72fdae4256902f407e9a6c11dac2aL341 into a separate try block?

matthiasschaub commented 2 months ago

In general, it would be nice to do only one thing in the try-block. Then it is easier to parse the exceptions. What can go wrong where.

You suggest placing line 341 https://github.com/GIScience/ohsome-py/pull/164/files#diff-0684a040fd4741e6267194be04e688f118c72fdae4256902f407e9a6c11dac2aL341 into a separate try block?

Yes, or even:

response = self._session().post(url=self._url, data=self._parameters)
try:
    response.raise_for_status(
except:
    ...
try:
    response.json()
except:
    ...

So that one knows that exceptions caught in one block belong only to the http response or json parsing and not to _session().post.

SlowMo24 commented 1 month ago

In general, it would be nice to do only one thing in the try-block. Then it is easier to parse the exceptions. What can go wrong where.

You suggest placing line 341 https://github.com/GIScience/ohsome-py/pull/164/files#diff-0684a040fd4741e6267194be04e688f118c72fdae4256902f407e9a6c11dac2aL341 into a separate try block?

Yes, or even:
response = self._session().post(url=self._url, data=self._parameters)
try:
    response.raise_for_status(
except:
    ...
try:
    response.json()
except:
    ...
So that one knows that exceptions caught in one block belong only to the http response or json parsing and not to _session().post.

thanks, I had the same gut feeling during coding but thought it was too complicated. I have made this change now, please re-review

GIScience / ohsome-py

fix(response): reoder exception handling to correctly parse a broken response #164