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.

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

Jump to bottom

Expose the time at which the Rate Limit will reset #219

Merged
merged 5 commits into from
Jun 24, 2020
Merged

Expose the time at which the Rate Limit will reset #219

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
db2bd21
handle rate limit errors exposing the reset header value
lbalmaceda Jun 23, 2020
85e2336
add tests for rate limit
lbalmaceda Jun 23, 2020
fba7a5a
add test case for missing headers
lbalmaceda Jun 23, 2020
0741fac
add an error handling note to the readme
lbalmaceda Jun 23, 2020
3df10de
Apply suggestions from code review
lbalmaceda Jun 24, 2020
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
10 changes: 10 additions & 0 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,16 @@ With all in place, the next snippets shows how to verify an RS256 signed ID toke
Provided something goes wrong, a ``TokenValidationError`` will be raised. In this
scenario, the ID token should be deemed invalid and its contents not be trusted.

==============
Error Handling
==============

When consuming methods from the API clients, the requests could fail for a number of reasons:
- Invalid data sent as part of the request: An ``Auth0Error` is raised with the error code and description.
- Global or Client Rate Limit reached: A ``RateLimitError`` is raised and the time at which the limit
resets is exposed in the ``reset_at`` property. When the header is unset, this value will be ``-1``.
- Network timeouts: Adjustable by passing a the ``timeout`` argument to the client. See the `rate limit docs <https://auth0.com/docs/policies/rate-limits>`_ for details.
lbalmaceda marked this conversation as resolved.
Show resolved Hide resolved

Available Management Endpoints
==============================

Expand Down
19 changes: 12 additions & 7 deletions auth0/v3/authentication/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
import sys
import platform
import requests
from ..exceptions import Auth0Error

from ..exceptions import Auth0Error, RateLimitError

UNKNOWN_ERROR = 'a0.sdk.internal.unknown'


class AuthenticationBase(object):

"""Base authentication object providing simple REST methods.

Args:
Expand Down Expand Up @@ -69,12 +67,19 @@ def _parse(self, response):


class Response(object):
def __init__(self, status_code, content):
def __init__(self, status_code, content, headers):
self._status_code = status_code
self._content = content
self._headers = headers

def content(self):
if self._is_error():
if self._status_code == 429:
reset_at = int(self._headers.get('x-ratelimit-reset', '-1'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the default of -1 is being set here

raise RateLimitError(error_code=self._error_code(),
message=self._error_message(),
reset_at=reset_at)

raise Auth0Error(status_code=self._status_code,
error_code=self._error_code(),
message=self._error_message())
Expand All @@ -95,7 +100,7 @@ def _error_message(self):
class JsonResponse(Response):
def __init__(self, response):
content = json.loads(response.text)
super(JsonResponse, self).__init__(response.status_code, content)
super(JsonResponse, self).__init__(response.status_code, content, response.headers)

def _error_code(self):
if 'error' in self._content:
Expand All @@ -111,7 +116,7 @@ def _error_message(self):

class PlainResponse(Response):
def __init__(self, response):
super(PlainResponse, self).__init__(response.status_code, response.text)
super(PlainResponse, self).__init__(response.status_code, response.text, response.headers)

def _error_code(self):
return UNKNOWN_ERROR
Expand All @@ -122,7 +127,7 @@ def _error_message(self):

class EmptyResponse(Response):
def __init__(self, status_code):
super(EmptyResponse, self).__init__(status_code, '')
super(EmptyResponse, self).__init__(status_code, '', {})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems like a good default for a successful but empty response


def _error_code(self):
return UNKNOWN_ERROR
Expand Down
6 changes: 6 additions & 0 deletions auth0/v3/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,11 @@ def __str__(self):
return '{}: {}'.format(self.status_code, self.message)


class RateLimitError(Auth0Error):
def __init__(self, error_code, message, reset_at):
super(RateLimitError, self).__init__(status_code=429, error_code=error_code, message=message)
self.reset_at = reset_at


class TokenValidationError(Exception):
pass
26 changes: 17 additions & 9 deletions auth0/v3/management/rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import sys
import platform
import json
import base64
import json
import platform
import sys

import requests
from ..exceptions import Auth0Error

from ..exceptions import Auth0Error, RateLimitError

UNKNOWN_ERROR = 'a0.sdk.internal.unknown'


class RestClient(object):

"""Provides simple methods for handling all RESTful api endpoints.

Args:
Expand All @@ -21,6 +21,7 @@ class RestClient(object):
both values separately or a float to set both to it.
(defaults to 5.0 for both)
"""

def __init__(self, jwt, telemetry=True, timeout=5.0):
self.jwt = jwt
self.timeout = timeout
Expand Down Expand Up @@ -96,12 +97,19 @@ def _parse(self, response):


class Response(object):
def __init__(self, status_code, content):
def __init__(self, status_code, content, headers):
self._status_code = status_code
self._content = content
self._headers = headers

def content(self):
if self._is_error():
if self._status_code == 429:
reset_at = int(self._headers.get('x-ratelimit-reset', '-1'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

headers once parsed by the library are put into a CaseInsensitiveDict. Also, headers are by spec case insensitive -> https://requests.readthedocs.io/en/master/user/quickstart/#response-headers

raise RateLimitError(error_code=self._error_code(),
message=self._error_message(),
reset_at=reset_at)

raise Auth0Error(status_code=self._status_code,
error_code=self._error_code(),
message=self._error_message())
Expand All @@ -122,7 +130,7 @@ def _error_message(self):
class JsonResponse(Response):
def __init__(self, response):
content = json.loads(response.text)
super(JsonResponse, self).__init__(response.status_code, content)
super(JsonResponse, self).__init__(response.status_code, content, response.headers)

def _error_code(self):
if 'errorCode' in self._content:
Expand All @@ -141,7 +149,7 @@ def _error_message(self):

class PlainResponse(Response):
def __init__(self, response):
super(PlainResponse, self).__init__(response.status_code, response.text)
super(PlainResponse, self).__init__(response.status_code, response.text, response.headers)

def _error_code(self):
return UNKNOWN_ERROR
Expand All @@ -152,7 +160,7 @@ def _error_message(self):

class EmptyResponse(Response):
def __init__(self, status_code):
super(EmptyResponse, self).__init__(status_code, '')
super(EmptyResponse, self).__init__(status_code, '', {})
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you not expect the API endpoints that return empty responses to get rate limited?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see - so if there's no response.text we know it's been successful so no need to check the rate limit header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Responses sent from the server could be:

  • 204 NO CONTENT (empty body, empty text)
  • Plain text response: e.g. "User is blocked"
  • JSON response: e.g. { "code": "success" }

Every Response instance of this SDK defines a method to check if they represent errors, by checking against the status code here
https://github.com/auth0/auth0-python/blob/master/auth0/v3/management/rest.py#L111-L112.

Since for now, the headers are only being used for rate limit purposes, and only in the case of an errored response, I think it makes sense to pass an empty dict here. In addition, this headers property in the Response is not accessible for developers since the parsed content is what is being returned after a successful call, or an exception is raised if this represents an error.

https://github.com/auth0/auth0-python/blob/master/auth0/v3/management/rest.py#L84-L87

Copy link
Contributor Author

Choose a reason for hiding this comment

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

replying to your question, 429 responses have been returning a non-empty body. I don't expect empty responses to represent a rate limit exception ever. 👍


def _error_code(self):
return UNKNOWN_ERROR
Expand Down
56 changes: 49 additions & 7 deletions auth0/v3/test/authentication/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import requests
import unittest
from ...authentication.base import AuthenticationBase
from ...exceptions import Auth0Error
from ...exceptions import Auth0Error, RateLimitError


class TestBase(unittest.TestCase):
Expand All @@ -27,7 +27,7 @@ def test_telemetry_enabled_by_default(self):
sys.version_info.micro)

client_info = {
'name': 'auth0-python',
'name': 'auth0-python',
'version': auth0_version,
'env': {
'python': python_version
Expand All @@ -53,10 +53,10 @@ def test_post(self, mock_post):
data = ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'})

mock_post.assert_called_with(url='the-url', json={'a': 'b'},
headers={'c': 'd', 'Content-Type': 'application/json'}, timeout=(10, 2))
headers={'c': 'd', 'Content-Type': 'application/json'}, timeout=(10, 2))

self.assertEqual(data, {'x': 'y'})

@mock.patch('requests.post')
def test_post_with_defaults(self, mock_post):
ab = AuthenticationBase('auth0.com', telemetry=False)
Expand All @@ -68,7 +68,7 @@ def test_post_with_defaults(self, mock_post):
data = ab.post('the-url')

mock_post.assert_called_with(url='the-url', json=None,
headers={'Content-Type': 'application/json'}, timeout=5.0)
headers={'Content-Type': 'application/json'}, timeout=5.0)

self.assertEqual(data, {'x': 'y'})

Expand Down Expand Up @@ -109,6 +109,48 @@ def test_post_error(self, mock_post):
self.assertEqual(context.exception.error_code, 'e0')
self.assertEqual(context.exception.message, 'desc')

@mock.patch('requests.post')
def test_post_rate_limit_error(self, mock_post):
ab = AuthenticationBase('auth0.com', telemetry=False)

mock_post.return_value.text = '{"statusCode": 429,' \
' "error": "e0",' \
' "error_description": "desc"}'
mock_post.return_value.status_code = 429
mock_post.return_value.headers = {
'x-ratelimit-limit': '3',
'x-ratelimit-remaining': '6',
'x-ratelimit-reset': '9',
}

with self.assertRaises(Auth0Error) as context:
ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'})

self.assertEqual(context.exception.status_code, 429)
self.assertEqual(context.exception.error_code, 'e0')
self.assertEqual(context.exception.message, 'desc')
self.assertIsInstance(context.exception, RateLimitError)
self.assertEqual(context.exception.reset_at, 9)

@mock.patch('requests.post')
def test_post_rate_limit_error_without_headers(self, mock_post):
ab = AuthenticationBase('auth0.com', telemetry=False)

mock_post.return_value.text = '{"statusCode": 429,' \
' "error": "e0",' \
' "error_description": "desc"}'
mock_post.return_value.status_code = 429
mock_post.return_value.headers = {}

with self.assertRaises(Auth0Error) as context:
ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'})

self.assertEqual(context.exception.status_code, 429)
self.assertEqual(context.exception.error_code, 'e0')
self.assertEqual(context.exception.message, 'desc')
self.assertIsInstance(context.exception, RateLimitError)
self.assertEqual(context.exception.reset_at, -1)

@mock.patch('requests.post')
def test_post_error_with_code_property(self, mock_post):
ab = AuthenticationBase('auth0.com', telemetry=False)
Expand Down Expand Up @@ -184,7 +226,7 @@ def test_get(self, mock_get):
data = ab.get('the-url', params={'a': 'b'}, headers={'c': 'd'})

mock_get.assert_called_with(url='the-url', params={'a': 'b'},
headers={'c': 'd', 'Content-Type': 'application/json'}, timeout=(10, 2))
headers={'c': 'd', 'Content-Type': 'application/json'}, timeout=(10, 2))

self.assertEqual(data, {'x': 'y'})

Expand All @@ -199,7 +241,7 @@ def test_get_with_defaults(self, mock_get):
data = ab.get('the-url')

mock_get.assert_called_with(url='the-url', params=None,
headers={'Content-Type': 'application/json'}, timeout=5.0)
headers={'Content-Type': 'application/json'}, timeout=5.0)

self.assertEqual(data, {'x': 'y'})

Expand Down
57 changes: 49 additions & 8 deletions auth0/v3/test/management/test_rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import requests

from ...management.rest import RestClient
from ...exceptions import Auth0Error
from ...exceptions import Auth0Error, RateLimitError


class TestRest(unittest.TestCase):
Expand Down Expand Up @@ -149,6 +149,48 @@ def test_get_errors(self, mock_get):
self.assertEqual(context.exception.error_code, 'code')
self.assertEqual(context.exception.message, 'message')

@mock.patch('requests.get')
def test_get_rate_limit_error(self, mock_get):
rc = RestClient(jwt='a-token', telemetry=False)

mock_get.return_value.text = '{"statusCode": 429,' \
' "errorCode": "code",' \
' "message": "message"}'
mock_get.return_value.status_code = 429
mock_get.return_value.headers = {
'x-ratelimit-limit': '3',
'x-ratelimit-remaining': '6',
'x-ratelimit-reset': '9',
}

with self.assertRaises(Auth0Error) as context:
rc.get('the/url')

self.assertEqual(context.exception.status_code, 429)
self.assertEqual(context.exception.error_code, 'code')
self.assertEqual(context.exception.message, 'message')
self.assertIsInstance(context.exception, RateLimitError)
self.assertEqual(context.exception.reset_at, 9)

@mock.patch('requests.get')
def test_get_rate_limit_error_without_headers(self, mock_get):
rc = RestClient(jwt='a-token', telemetry=False)

mock_get.return_value.text = '{"statusCode": 429,' \
' "errorCode": "code",' \
' "message": "message"}'
mock_get.return_value.status_code = 429

mock_get.return_value.headers = {}
with self.assertRaises(Auth0Error) as context:
rc.get('the/url')

self.assertEqual(context.exception.status_code, 429)
self.assertEqual(context.exception.error_code, 'code')
self.assertEqual(context.exception.message, 'message')
self.assertIsInstance(context.exception, RateLimitError)
self.assertEqual(context.exception.reset_at, -1)

@mock.patch('requests.post')
def test_post(self, mock_post):
rc = RestClient(jwt='a-token', telemetry=False)
Expand Down Expand Up @@ -312,7 +354,6 @@ def test_file_post_content_type_is_none(self, mock_post):

mock_post.assert_called_once_with('the-url', data=data, files=files, headers=headers, timeout=5.0)


@mock.patch('requests.put')
def test_put(self, mock_put):
rc = RestClient(jwt='a-token', telemetry=False)
Expand All @@ -326,7 +367,7 @@ def test_put(self, mock_put):

response = rc.put(url='the-url', data=data)
mock_put.assert_called_with('the-url', json=data,
headers=headers, timeout=5.0)
headers=headers, timeout=5.0)

self.assertEqual(response, ['a', 'b'])

Expand All @@ -335,8 +376,8 @@ def test_put_errors(self, mock_put):
rc = RestClient(jwt='a-token', telemetry=False)

mock_put.return_value.text = '{"statusCode": 999,' \
' "errorCode": "code",' \
' "message": "message"}'
' "errorCode": "code",' \
' "message": "message"}'
mock_put.return_value.status_code = 999

with self.assertRaises(Auth0Error) as context:
Expand Down Expand Up @@ -407,7 +448,7 @@ def test_delete_with_body_and_params(self, mock_delete):
mock_delete.return_value.status_code = 200

data = {'some': 'data'}
params={'A': 'param', 'B': 'param'}
params = {'A': 'param', 'B': 'param'}

response = rc.delete(url='the-url/ID', params=params, data=data)
mock_delete.assert_called_with('the-url/ID', headers=headers, params=params, json=data, timeout=5.0)
Expand Down Expand Up @@ -436,7 +477,7 @@ def test_disabled_telemetry(self):
'Content-Type': 'application/json',
'Authorization': 'Bearer a-token',
}

self.assertEqual(rc.base_headers, expected_headers)

def test_enabled_telemetry(self):
Expand All @@ -454,7 +495,7 @@ def test_enabled_telemetry(self):
sys.version_info.micro)

client_info = {
'name': 'auth0-python',
'name': 'auth0-python',
'version': auth0_version,
'env': {
'python': python_version
Expand Down