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

feat: Introduce a new standartized drf errors format #5223

Closed
wants to merge 15 commits into from

Conversation

iskhakov
Copy link
Contributor

@iskhakov iskhakov commented Nov 1, 2024

What this PR does

This pull request aims to standardize error handling in the Django Rest Framework by implementing the drf_standardized_errors package. The changes involve:

  • Adding a new error formatter class that combines the old and new error response formats to maintain compatibility.
  • Updating multiple API views to raise standardized exceptions instead of returning error responses directly.

Which issue(s) this PR closes

Related to [issue link here]

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

Comment on lines +16 to +24
def format_error_response(self, error_response: ErrorResponse):
old_error_response_data = drf_exception_handler(self.exc, self.context).data
# For the compatibility reasons, we are keeping the old format of error response
# and adding the new format of error response in the same response.
# This is done to avoid breaking the existing clients.
# New format uses the default error response format from drf_standardized_errors.
new_error_response_data = super().format_error_response(error_response)
data = {**old_error_response_data, "new_format_data": new_error_response_data}
return data
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here is the main idea of the PR

@@ -2149,7 +2149,7 @@ def test_alert_group_resolve_resolution_note(
response = client.post(url, format="json", **make_user_auth_headers(user, token))
# check that resolution note is required
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json()["code"] == AlertGroupAPIError.RESOLUTION_NOTE_REQUIRED.value
assert response.json()["code"] == str(AlertGroupAPIError.RESOLUTION_NOTE_REQUIRED.value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the error is raised as an exception, only string is possible

@iskhakov iskhakov added the pr:no public docs Added to a PR that does not require public documentation updates label Nov 4, 2024
@iskhakov iskhakov changed the title Standartize drf errors feat: Introduce a new standartized drf errors format Nov 4, 2024
@iskhakov
Copy link
Contributor Author

some of the exceptions that are raised outside of drf are not handled by drf-standartized-errors handler, especially in middleware. We need to address that before rolling this pr. Closing the PR for now

@iskhakov iskhakov closed this Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant