diff --git a/README.md b/README.md index e2bede7..01e9179 100644 --- a/README.md +++ b/README.md @@ -37,10 +37,13 @@ Django styleguide that we use in [HackSoft](https://hacksoft.io). * [Nested serializers](#nested-serializers) * [Advanced serialization](#advanced-serialization) - [Urls](#urls) -- [Exception Handling](#exception-handling) - * [Raising Exceptions in Services / Selectors](#raising-exceptions-in-services--selectors) - * [Handle Exceptions in APIs](#handle-exceptions-in-apis) - * [Error formatting](#error-formatting) +- [Errors & Exception Handling](#errors--exception-handling) + * [How exception handling works (in the context of DRF)](#how-exception-handling-works-in-the-context-of-drf) + + [DRF's `ValidationError`](#drfs-validationerror) + + [Django's `ValidationError`](#djangos-validationerror) + * [Describe how your API errors are going to look like.](#describe-how-your-api-errors-are-going-to-look-like) + * [Know how to change the default exception handling behavior.](#know-how-to-change-the-default-exception-handling-behavior) + * [Approach 1 - Use DRF's default exceptions, with very little modifications.](#approach-1---use-drfs-default-exceptions-with-very-little-modifications) - [Testing](#testing-2) * [Naming conventions](#naming-conventions) - [Celery](#celery) @@ -1014,250 +1017,404 @@ urlpatterns = [ **Splitting urls like that can give you the extra flexibility to move separate domain patterns to separate modules**, especially for really big projects, where you'll often have merge conflicts in `urls.py`. -## Exception Handling +## Errors & Exception Handling -### Raising Exceptions in Services / Selectors +Errors & exception handling is a big topic & quite often - the details are specific for a given project. -Now we have a separation between our HTTP interface & the core logic of our application. +That's why we'll split things into 2 - **general guidelines**, followed by some **specific approaches** for error handling. -To keep this separation of concerns, our services and selectors must not use the `rest_framework.exception` classes because they are bounded with HTTP status codes. +Our general guidelines are: -Our services and selectors must use one of: +1. Know how exception handling works (we'll give context for Django Rest Framework). +1. Describe how your API errors are going to look like. +1. Know how to change the default exception handling behavior. -* [Python built-in exceptions](https://docs.python.org/3/library/exceptions.html) -* Exceptions from `django.core.exceptions` -* Custom exceptions, inheriting from the ones above. +Followed by some specific approaches: -Here is a good example of service that performs some validation and raises `django.core.exceptions.ValidationError`: +1. Use DRF's default exceptions, with very little modifications. +1. Validation throws `ValidationError`, business logic throws `ApplicationError`. + +### How exception handling works (in the context of DRF) + +DRF has an excellent guide on how exceptions are being handled, so make sure to read it first - + +Additionally, we love to visualize with diagrams, so here's an overview of the process: + +![Exception handler (1)](https://user-images.githubusercontent.com/387867/142426205-2c0356e6-ce20-425e-a811-072c3334edb0.png) + +Basically, if the exception handler cannot handle the given exception & returns `None`, this will result in an unhandled exception & a `500 Server Error`. This is often good, because you won't be silencing errors, that you need to pay attention to. + +**Now, there are some quirks, that we need to pay attention to.** + +#### DRF's `ValidationError` + +For example, if we simply raise a `rest_framework.exceptions.ValidationError` like that: ```python -from django.core.exceptions import ValidationError +from rest_framework import exceptions -def create_topic(*, name: str, course: Course) -> Topic: - if course.end_date < timezone.now(): - raise ValidationError('You can not create topics for course that has ended.') - - topic = Topic.objects.create(name=name, course=course) - - return topic +def some_service(): + raise ValidationError("Error message here.") ``` -### Handle Exceptions in APIs +The response payload is going to look like this: -To transform the exceptions raised in the services or selectors, to a standard HTTP response, you need to catch the exception and raise something that the rest framework understands. - -The best place to do this is in the `handle_exception` method of the `APIView`. There you can map your Python/Django exception to a DRF exception. - -By default, the [`handle_exception` method implementation in DRF](https://www.django-rest-framework.org/api-guide/exceptions/#exception-handling-in-rest-framework-views) handles Django's built-in `Http404` and `PermissionDenied` exceptions, thus there is no need for you to handle it by hand. - -Here is an example: - -```python -from rest_framework import exceptions as rest_exceptions - -from django.core.exceptions import ValidationError - - -class CourseCreateApi(SomeAuthenticationMixin, APIView): - expected_exceptions = { - ValidationError: rest_exceptions.ValidationError - } - - class InputSerializer(serializers.Serializer): - ... - - def post(self, request): - serializer = self.InputSerializer(data=request.data) - serializer.is_valid(raise_exception=True) - - create_course(**serializer.validated_data) - - return Response(status=status.HTTP_201_CREATED) - - def handle_exception(self, exc): - if isinstance(exc, tuple(self.expected_exceptions.keys())): - drf_exception_class = self.expected_exceptions[exc.__class__] - drf_exception = drf_exception_class(get_error_message(exc)) - - return super().handle_exception(drf_exception) - - return super().handle_exception(exc) -``` - -Here's the implementation of `get_error_message`: - -```python -def get_first_matching_attr(obj, *attrs, default=None): - for attr in attrs: - if hasattr(obj, attr): - return getattr(obj, attr) - - return default - - -def get_error_message(exc): - if hasattr(exc, 'message_dict'): - return exc.message_dict - error_msg = get_first_matching_attr(exc, 'message', 'messages') - - if isinstance(error_msg, list): - error_msg = ', '.join(error_msg) - - if error_msg is None: - error_msg = str(exc) - - return error_msg -``` - -You can move this code to a mixin and use it in every API to prevent code duplication. - -We call this `ApiErrorsMixin`. Here's a sample implementation from one of our projects: - -```python -from rest_framework import exceptions as rest_exceptions - -from django.core.exceptions import ValidationError - -from project.common.utils import get_error_message - - -class ApiErrorsMixin: - """ - Mixin that transforms Django and Python exceptions into rest_framework ones. - Without the mixin, they return 500 status code which is not desired. - """ - expected_exceptions = { - ValueError: rest_exceptions.ValidationError, - ValidationError: rest_exceptions.ValidationError, - PermissionError: rest_exceptions.PermissionDenied - } - - def handle_exception(self, exc): - if isinstance(exc, tuple(self.expected_exceptions.keys())): - drf_exception_class = self.expected_exceptions[exc.__class__] - drf_exception = drf_exception_class(get_error_message(exc)) - - return super().handle_exception(drf_exception) - - return super().handle_exception(exc) -``` - -Having this mixin in mind, our API can be written like that: - -```python -class CourseCreateApi( - SomeAuthenticationMixin, - ApiErrorsMixin, - APIView -): - class InputSerializer(serializers.Serializer): - ... - - def post(self, request): - serializer = self.InputSerializer(data=request.data) - serializer.is_valid(raise_exception=True) - - create_course(**serializer.validated_data) - - return Response(status=status.HTTP_201_CREATED) -``` - -All of the code above can be found in [here](https://github.com/HackSoftware/Styleguide-Example/blob/master/styleguide_example/api/mixins.py#L70), in the [Styleguide-Example](https://github.com/HackSoftware/Styleguide-Example) repo. - -### Error formatting - -The next step is to generalize the format of the errors we get from our APIs. This will ease the process of displaying errors to the end-user, via JavaScript. - -If we have a standard serializer and there is an error with one of the fields, the message we get by default looks like this: - -```python -{ - "url": [ - "This field is required." - ] -} -``` - -If we have a validation error with just a message - `raise ValidationError('Something is wrong.')` - it will look like this: - -```python +```json [ - "some error" + "Some message" ] ``` -Another error format may look like this: +This looks strange, because if we do it like this: ```python +from rest_framework import exceptions + + +def some_service(): + raise exceptions.ValidationError({"error": "Some message"}) +``` + +The response payload is going to look like this: + +```json { - "detail": "Method \"GET\" not allowed." + "error": "Some message" } ``` -**Those are 3 different ways of formatting for our errors.** What we want to have is a single format, for all errors. +That's basically what we passed as the `detail` of the `ValidationError`. But it's a different data structure from the initial array. -Luckily, DRF provides a way for us to give our own custom exception handler, where we can implement the desired formatting: - -In our projects, we format the errors like that: +Now, if we decide to raise another of the DRF's built-in exceptions: ```python +from rest_framework import exceptions + + +def some_service(): + raise exceptions.NotFound() +``` + +The response payload is going to look like this: + +```json { - "errors": [ - { - "message": "Error message", - "code": "Some code", - "field": "field_name" - }, - { - "message": "Error message", - "code": "Some code", - "field": "nested.field_name" - }, - ] + "detail": "Not found." } ``` -If we raise a `ValidationError`, then the field is optional. +That's entirely different from what we saw as behavior from the `ValidationError` and this might cause problems. -In order to achieve that, we implement a custom exception handler: +So far, the default behavior can get us: + +- An array. +- A dictionarry. +- A specific `{"detail": "something"}` result. + +#### Django's `ValidationError` + +Now, DRF's default exception handling is not playing nice with Django's `ValidationError`. + +This piece of code: ```python +from django.core.exceptions import ValidationError as DjangoValidationError + + +def some_service(): + raise DjangoValidationError("Some error message") +``` + +Will result in an unhandled exception, causing `500 Server Error`. + +This will also happen if this `ValidationError` comes from model validation, for example: + +```python +def some_service(): + user = BaseUser() + user.full_clean() # Throws ValidationError + user.save() +``` + +This will also result in `500 Server Error`. + +If we want to start handling this, as if it was `rest_framework.exceptions.ValidationError`, we need to roll-out our own [custom exception handler](https://www.django-rest-framework.org/api-guide/exceptions/#custom-exception-handling): + +```python +from django.core.exceptions import ValidationError as DjangoValidationError + from rest_framework.views import exception_handler +from rest_framework.serializers import as_serializer_error +from rest_framework import exceptions -def exception_errors_format_handler(exc, context): - response = exception_handler(exc, context) +def custom_exception_handler(exc, ctx): + if isinstance(exc, DjangoValidationError): + exc = exceptions.ValidationError(as_serializer_error(exc)) - # If an unexpected error occurs (server error, etc.) + response = exception_handler(exc, ctx) + + # If unexpected error occurs (server error, etc.) if response is None: return response - formatter = ErrorsFormatter(exc) - - response.data = formatter() - return response ``` -which needs to be added to the `REST_FRAMEWORK` project settings: +This is basically the default implementation, with the addition of this piece of code: ```python -REST_FRAMEWORK = { - 'EXCEPTION_HANDLER': 'project.app.handlers.exception_errors_format_handler', - ... +if isinstance(exc, DjangoValidationError): + exc = exceptions.ValidationError(as_serializer_error(exc)) +``` + +Now, since we need to map between `django.core.exceptions.ValidationError` and `rest_framework.exceptions.ValidationError`, we are using DRF's `as_serializer_error`, which is used internally in the serializers, just for that. + +With that, we can now have Django's `ValidationError` playing nice with DRF's exception handler. + +### Describe how your API errors are going to look like. + +This is very important and should be done as early as possible in any given project. + +This is basically agreening upon what the interface of your API errors - how an error is going to look like? + +This is very project specific, you can use some of the popular APIs for inspiration: + +- Stripe - + +As an example, we might decide that our errors are going to look like this: + +1. `4**` and `5**` status codes for different types of errors. +1. Each error will be a dictionarry with a single `message` key, containing the error message. + +```json +{ + "message": "Some error message here" } ``` -**The magic happens in the `ErrorsFormatter` class.** +That's simple enough: -The implementation of that class can be found [here](https://github.com/HackSoftware/Styleguide-Example/blob/master/styleguide_example/api/errors.py), in the [Styleguide-Example](https://github.com/HackSoftware/Styleguide-Example) repo. +- `400` will be used for validation errors. +- `401` for auth errors. +- `403` for permission errors. +- `404` for not found errors. +- `429` for throttling errors. +- `500` for server errors (we need to be careful not to silence an exception causing 500 and always report that in services like Sentry) -Combining `ApiErrorsMixin`, the custom exception handler & the errors formatter class, we can have predictable behavior in our APIs, when it comes to errors. +Again, this is up to you & it's specific to the project. **We'll propose something similiar for one of the specific approaches.** -**A note:** +### Know how to change the default exception handling behavior. -> We've moved away from this particular way of formatting errors & we'll be updating the styleguide with a more generic approach. +This is also important, because when you decide how your errors are going to look like, you need to implement this as custom exception handling. + +We've already provided an example for that in the paragraph above, talking about Django's `ValidationError`. + +We'll also provide additional examples in the sections below. + +### Approach 1 - Use DRF's default exceptions, with very little modifications. + +DRF's error handling is good. It'd be great, if the end result was always consistent. Those are the little modifications that we are going to do. + +We want to end up with errors, always looking like that: + +```json +{ + "detail": "Some error" +} +``` + +or + +```json +{ + "detail": ["Some error", "Another error"] +} +``` + +or + +```json +{ + "detail": { "key": "... some arbitrary nested structure ..." } +} +``` + +Basically, make sure we always have a dictionary with a `detail` key. + +Additonally, we want to handle Django's `ValidationError` as well. + +In order to achieve that, this is how our custom exception handler is going to look like: + +```python +def drf_default_with_modifications_exception_handler(exc, ctx): + if isinstance(exc, DjangoValidationError): + exc = exceptions.ValidationError(as_serializer_error(exc)) + + response = exception_handler(exc, ctx) + + # If unexpected error occurs (server error, etc.) + if response is None: + return response + + if isinstance(exc.detail, (list, dict)): + response.data = { + "detail": response.data + } + + return response +``` + +Now, lets run a set of tests: + +Code: + +```python +def some_service(): + raise DjangoValidationError("Some error message") +``` + +Response: + +```json +{ + "detail": { + "non_field_errors": [ + "Some error message" + ] + } +} +``` + +--- + +Code: + +```python +def some_service(): + raise RestValidationError("Some error message") +``` + +Response: + +```json +{ + "detail": [ + "Some error message" + ] +} +``` + +--- + +Code: + +```python +def some_service(): + raise RestValidationError(detail={"error": "Some error message"}) +``` + +Response: + +```json +{ + "detail": { + "error": "Some error message" + } +} +``` + +--- + +Code: + +```python +class NestedSerializer(serializers.Serializer): + bar = serializers.CharField() + + +class PlainSerializer(serializers.Serializer): + foo = serializers.CharField() + email = serializers.EmailField(min_length=200) + + nested = NestedSerializer() + + +def some_service(): + serializer = PlainSerializer(data={ + "email": "foo", + "nested": {} + }) + serializer.is_valid(raise_exception=True) + +``` + +Response: + +```json +{ + "detail": { + "foo": [ + "This field is required." + ], + "email": [ + "Ensure this field has at least 200 characters.", + "Enter a valid email address." + ], + "nested": { + "bar": [ + "This field is required." + ] + } + } +} +``` + +--- + +Code: + +```python +from rest_framework import exceptions + + +def some_service(): + raise exceptions.Throttled() +``` + +Response: + +```json +{ + "detail": "Request was throttled." +} +``` + +--- + +Code: + +```python +def some_service(): + user = BaseUser() + user.full_clean() +``` + +Response: + +```json +{ + "detail": { + "password": [ + "This field cannot be blank." + ], + "email": [ + "This field cannot be blank." + ] + } +} +``` ## Testing