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

Better exception #355

Merged
merged 3 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
1.4 (unreleased)
================

- More explicit exception messages where the payload is
sometimes considered if it can be presented properly (simple
body text or job error message). [#355]

- we now ignore namespaces in xsi-type attributes; this is a lame fix
for services like ESO's and MAST's TAP, which do not use canonical
prefixes while astropy.utils.xml ignores namespaces. [#323]
Expand Down
9 changes: 6 additions & 3 deletions pyvo/dal/adhoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import numpy as np
import warnings
import copy
import requests
from collections import OrderedDict

from .query import DALResults, DALQuery, DALService, Record
Expand Down Expand Up @@ -247,7 +248,10 @@ def getdataset(self, timeout=None):
try:
url = next(self.getdatalink().bysemantics('#this')).access_url
response = self._session.get(url, stream=True, timeout=timeout)
response.raise_for_status()
try:
response.raise_for_status()
except requests.RequestException as ex:
raise DALServiceError.from_except(ex, url)
return response.raw
except (DALServiceError, ValueError, StopIteration):
# this should go to Record.getdataset()
Expand Down Expand Up @@ -563,8 +567,7 @@ def bysemantics(self, semantics, include_narrower=True):
additional_terms.extend(voc["terms"][term]["narrower"])
core_terms = core_terms+additional_terms

semantics = set("#"+term for term in core_terms
) | set(other_terms)
semantics = set("#"+term for term in core_terms) | set(other_terms)
for record in self:
if record.semantics in semantics:
yield record
Expand Down
13 changes: 10 additions & 3 deletions pyvo/dal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,18 @@ def from_except(cls, exc, url=None):
for the given exception that represents the underlying cause.
"""
if isinstance(exc, requests.exceptions.RequestException):
message = str(exc)
try:
code = exc.response.status_code
response = exc.response
except AttributeError:
code = 0
response = None
code = 0
if response is not None:
code = response.status_code
message = str(exc)
content_type = response.headers.get('content-type', None)
if content_type and 'text/plain' in content_type:
message = '{} for {}'.format(response.text, url)
bsipocz marked this conversation as resolved.
Show resolved Hide resolved
# TODO votable handling

return DALServiceError(message, code, exc, url)
elif isinstance(exc, Exception):
Expand Down
7 changes: 5 additions & 2 deletions pyvo/dal/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def __init__(self, votable, url=None, session=None):

if self._status[0].lower() == "overflow":
warn("Partial result set. Potential causes MAXREC, async storage space, etc.",
category=DALOverflowWarning);
category=DALOverflowWarning)

self._resultstable = self._findresultstable(votable)
if not self._resultstable:
Expand Down Expand Up @@ -771,8 +771,11 @@ def getdataset(self, timeout=None):
response = self._session.get(url, stream=True, timeout=timeout)
else:
response = self._session.get(url, stream=True)
try:
response.raise_for_status()
except requests.RequestException as ex:
raise DALServiceError.from_except(ex, url)

response.raise_for_status()
return response.raw

def cachedataset(self, filename=None, dir=".", timeout=None, bufsize=None):
Expand Down
3 changes: 2 additions & 1 deletion pyvo/dal/tap.py
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,8 @@ def raise_if_error(self):
if theres an error
"""
if self.phase in {"ERROR", "ABORTED"}:
raise DALQueryError("Query Error", self.phase, self.url)
msg = self._job._message if self._job and self._job._message else ''
raise DALQueryError("Query Error: " + msg, self.url)

def fetch_result(self):
"""
Expand Down
17 changes: 16 additions & 1 deletion pyvo/dal/tests/test_tap.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import requests_mock

from pyvo.dal.tap import escape, search, AsyncTAPJob, TAPService
from pyvo.dal import DALQueryError

from pyvo.io.uws import JobFile
from pyvo.io.uws.tree import Parameter, Result

Expand Down Expand Up @@ -98,7 +100,11 @@ def create(self, request, context):
job = JobFile()
job.version = "1.1"
job.jobid = newid
job.phase = 'PENDING'
if 'test_erroneus_submit.non_existent' in request.text:
job.phase = 'ERROR'
job.message = 'test_erroneus_submit.non_existent not found'
else:
job.phase = 'PENDING'
job.quote = Time.now() + TimeDelta(1, format='sec')
job.creationtime = Time.now()
job.executionduration = TimeDelta(3600, format='sec')
Expand Down Expand Up @@ -439,6 +445,15 @@ def test_submit_job(self):
job.wait()
job.delete()

@pytest.mark.usefixtures('async_fixture')
def test_erroneus_submit_job(self):
service = TAPService('http://example.com/tap')
job = service.submit_job(
"SELECT * FROM test_erroneus_submit.non_existent")
with pytest.raises(DALQueryError) as e:
job.raise_if_error()
assert 'test_erroneus_submit.non_existent not found' in str(e)

@pytest.mark.usefixtures('async_fixture')
def test_submit_job_case(self):
"""Test using mixed case in the QUERY parameter to a job.
Expand Down