Skip to content

Update client to handle unknown error formats and 401s #4

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 2, 2023
Merged

Update client to handle unknown error formats and 401s #4

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d75d495
Update client to handle unknown error formats and 401s
VDubber Jun 1, 2023
f3c15d3
update error msg to match UI
VDubber Jun 1, 2023
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
11 changes: 9 additions & 2 deletions jupiterone/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,19 @@ def _execute_query(self, query: str, variables: Dict = None) -> Dict:
raise JupiterOneApiError(content.get('errors'))
return response.json()

elif response.status_code == 401:
raise JupiterOneApiError('401: Unauthorized. Please supply a valid token')

elif response.status_code in [429, 500]:
raise JupiterOneApiRetryError('JupiterOne API rate limit exceeded')

else:
content = json.loads(response._content)
raise JupiterOneApiError('{}:{}'.format(response.status_code, content.get('error')))
try:
content = json.loads(response._content)
raise JupiterOneApiError('{}: {}'.format(response.status_code, content.get('error') or 'Unknown Error'))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit rusty with Python, but I think the or operator doesn't handle nullish coalescing like we'd expect in a TypeScript world. I'm assuming that's why we need the additional try/catch here to catch the ValueError?

We could do some kind of if statement optionally, but I think this solution is fine too.

Copy link
Author

@VDubber VDubber Jun 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I based this logic off of this post:
https://stackoverflow.com/questions/4978738/is-there-a-python-equivalent-of-the-c-sharp-null-coalescing-operator
And I was noticing that content.get was returning None which should be handled by or

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! So is the additional try/catch to handle something else, or more of a belt and suspenders kind of thing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Def belt and suspenders. The unknown handler :)

except ValueError as e:
raise JupiterOneApiError('{}: {}'.format(response.status_code, 'Unknown Error'));


def query_v1(self, query: str, **kwargs) -> Dict:
""" Performs a V1 graph query
Expand Down
27 changes: 27 additions & 0 deletions tests/test_query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,33 @@ def test_query_v1():
assert response['data'][0]['entity']['_id'] == '1'


@responses.activate
def test_401():
with pytest.raises(Exception) as ex:

def response_401_callback(r):
headers = {
'Content-Type': 'application/json'
}

response = {
'test': ['Unauthorized']
}
return (401, headers, json.dumps(response))

responses.add_callback(
responses.POST, 'https://api.us.jupiterone.io/graphql',
callback=response_401_callback,
content_type='application/text',
)

j1 = JupiterOneClient(account='testAccount', token='testToken')
query = "find Host with _id='1'"
j1.query_v1(query)

assert '401: Unauthorized. Please supply a valid token' in str(ex.value)


@responses.activate
def test_tree_query_v1():

Expand Down