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

fix: Added error handler for status 429 #6678

Merged
merged 29 commits into from Jan 13, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7e824fe
Added Error handler for status 429
Rits1272 Dec 19, 2019
dfcad5b
Revert "feat: Optimize marshamllow jsonapi link generation (#6656)" (…
kushthedude Dec 21, 2019
f6bd4b1
Revert "Revert "feat: Optimize marshamllow jsonapi link generation (#…
iamareebjamal Dec 21, 2019
f5c7235
fix: Order Post route view (#6683)
prateekj117 Dec 21, 2019
a00d19c
chore(deps): update geoip2 requirement from ~=2.9.0 to ~=3.0.0 (#6681)
dependabot-preview[bot] Dec 21, 2019
b6f948b
chore(deps): update celery requirement from ~=4.3 to ~=4.4 (#6670)
dependabot-preview[bot] Dec 21, 2019
d104ea2
chore(deps): update sqlalchemy requirement from ~=1.3.11 to ~=1… (#6671)
dependabot-preview[bot] Dec 21, 2019
09614e9
chore(deps): update coverage requirement from ~=4.5 to ~=5.0 (#6672)
dependabot-preview[bot] Dec 21, 2019
2305a56
Added error handler for status 429
Rits1272 Dec 22, 2019
849040a
Merge branch 'development' into development
Rits1272 Dec 22, 2019
5c65efc
Added error handler for status 429
Rits1272 Dec 22, 2019
9937e99
Merge branch 'development' of https://github.com/Rits1272/open-event-…
Rits1272 Dec 22, 2019
73b2b98
Added error handler for status 429
Rits1272 Dec 22, 2019
d71b9c5
Added error handler for status 429
Rits1272 Dec 22, 2019
f21a822
Merge branch 'development' into development
Rits1272 Dec 22, 2019
569a696
Merge branch 'master' of https://github.com/fossasia/open-event-serve…
Rits1272 Dec 23, 2019
9063705
Merge branch 'development' into development
Rits1272 Dec 23, 2019
9ef2e0d
Added error handler for status 429
Rits1272 Dec 23, 2019
cf80899
Merge branch 'development' of https://github.com/Rits1272/open-event-…
Rits1272 Dec 23, 2019
d0419ef
Added error handler for status 429
Rits1272 Dec 23, 2019
0c65e38
Merge branch 'development' of https://github.com/fossasia/open-event-…
Rits1272 Jan 3, 2020
4ca8411
Added error handler for status 429
Rits1272 Jan 3, 2020
3403d03
Merge branch 'development' into development
Rits1272 Jan 4, 2020
a6b749d
Merge branch 'development' into development
Rits1272 Jan 6, 2020
1584492
Merge branch 'development' into development
Rits1272 Jan 9, 2020
ffd433d
Added 429 error handler
Rits1272 Jan 9, 2020
9fb7911
Merge branch 'development' of https://github.com/Rits1272/open-event-…
Rits1272 Jan 9, 2020
2a1ec01
Added error handler for status 429
Rits1272 Jan 10, 2020
9c60849
Added error handler for status 429
Rits1272 Jan 10, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/__init__.py
Expand Up @@ -137,6 +137,7 @@ def create_app():
# development api
with app.app_context():
from app.api.admin_statistics_api.events import event_statistics
from app.api.helpers.errors import error_blueprint
from app.api.auth import auth_routes
from app.api.attendees import attendee_misc_routes
from app.api.bootstrap import api_v1
Expand Down Expand Up @@ -177,6 +178,7 @@ def create_app():
app.register_blueprint(ticket_blueprint)
app.register_blueprint(order_blueprint)
app.register_blueprint(event_blueprint)
app.register_blueprint(error_blueprint)

add_engine_pidguard(db.engine)

Expand Down
16 changes: 15 additions & 1 deletion app/api/helpers/errors.py
@@ -1,8 +1,9 @@
import json

Rits1272 marked this conversation as resolved.
Show resolved Hide resolved
from flask import make_response
from flask import make_response, Blueprint
from flask_rest_jsonapi.errors import jsonapi_errors

error_blueprint = Blueprint('errors', __name__)

class ErrorResponse:
"""
Expand Down Expand Up @@ -78,3 +79,16 @@ class BadRequestError(ErrorResponse):
"""
status = 400
title = 'Bad Request'

Rits1272 marked this conversation as resolved.
Show resolved Hide resolved

class RequestLimitError(ErrorResponse):
"""
Default class for 429 Error
"""
status = 429
title = 'Request Limit Exceeded'
Copy link
Member

Choose a reason for hiding this comment

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

Not needed. Try to understand the issue

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure



@error_blueprint.app_errorhandler(429)
def handle_429(error):
raise RequestLimitError({'source': ''}, error.description).respond()
Copy link
Member

Choose a reason for hiding this comment

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

Why this? Keep it consistent with other handlers

Copy link
Member Author

@Rits1272 Rits1272 Dec 20, 2019

Choose a reason for hiding this comment

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

Can you please tell me where other error handlers are implemented @kushthedude? Because I first tried to find how other handlers were registered but was not able to find them. Also, issue #6665 says to register custom error handlers that means previously no custom error handlers are registered. Please guide me further if I am going wrong

Copy link
Member

Choose a reason for hiding this comment

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

They are present in __init__.py of app

And you can't just raise in an error handler itself. The raise is caught in error handler so that proper response can be returned from there. The app will crash if you raise in the error handler itself which has caught the error, hence no point of catching at all

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok @iamareebjamal. Working on it. Thanks for reviewing!

7 changes: 6 additions & 1 deletion tests/all/integration/api/helpers/test_errors.py
Expand Up @@ -3,7 +3,7 @@

from tests.all.integration.utils import OpenEventTestCase
from app.api.helpers.errors import ErrorResponse, ForbiddenError, NotFoundError, ServerError, \
UnprocessableEntityError, BadRequestError
UnprocessableEntityError, BadRequestError, RequestLimitError
from tests.all.integration.setup_database import Setup
from flask_rest_jsonapi.errors import jsonapi_errors
from flask import make_response
Expand Down Expand Up @@ -47,6 +47,11 @@ def test_errors(self):
bad_request_error = BadRequestError({'source': ''}, 'Request cannot be served')
self.assertEqual(bad_request_error.status, 400)

# Request limit Error
request_limit_error = RequestLimitError({'source': ''},
'Request limit exceeded')
self.assertEqual(request_limit_error.status, 429)
Copy link
Member

Choose a reason for hiding this comment

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

Not needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Will remove it



if __name__ == '__main__':
unittest.main()