Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

WIP: Error handling #787

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

Closed
bpleshakov wants to merge 20 commits into django-json-api:master from bpleshakov:patch-5
Closed

Conversation

Copy link
Contributor

@bpleshakov bpleshakov commented May 13, 2020

Fixes error handling we discussed in #776

Description of the Change

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

Boris Pleshakov and others added 20 commits March 25, 2020 17:35
# Conflicts:
#	rest_framework_json_api/renderers.py
f json_api settings
f pytest.ini
...hat DeprecationWarning raises whenever nested serializer is used without correct settings
@bpleshakov bpleshakov changed the title (削除) Patch 5 (削除ここまで) (追記) WIP: Error handling (追記ここまで) May 13, 2020
Copy link
Member

sliverc commented Jun 28, 2020

@sapiosexual Do you think you will find some time in the near future to work on this PR again?

It would be good to have this merged before we release another version (which I would love to do soon). Let me know.



urlpatterns = [
path(r'^entries-nested/$', DummyTestView.as_view(),
Copy link
Contributor

Choose a reason for hiding this comment

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

regex not needed on path()

Copy link
Contributor Author

@sliverc I am about to start.
If nothing will go wrong and I won't get any sudden significant amount of work or any other kind of duties I will definitely start next week.

Copy link
Contributor

nattyg93 commented Jul 23, 2020
edited
Loading

@sapiosexual I'm keen to use your nested serializer feature, so it would be great to get something merged for exception handling. Is there anything I can do to help?

I am currently installing this package from git (commit: b14d8ab) into my project and substituting this exception handler below for the in built:

"""Exception handler with support for nested serializers."""
import inspect
from django.http import Http404
from rest_framework import exceptions
from rest_framework_json_api import utils
from rest_framework_json_api.exceptions import rendered_with_json_api
from rest_framework_json_api.settings import json_api_settings
def format_drf_errors(response, context, exc):
 """Format the drf error as a JSON:API error."""
 errors = []
 context["view"].resource_name = "errors"
 # handle generic errors. ValidationError('test') in a view for example
 if isinstance(response.data, list):
 for message in response.data:
 errors.append(utils.format_error_object(message, "/data", response))
 response.data = errors
 return errors
 # handle all errors thrown from serializers
 for field, error in response.data.items():
 errors.extend(_handle_error_value(response, exc, field, error))
 response.data = errors
 return response
def _handle_error_value(response, exc, field, error, key_prefix=None) -> list:
 field = utils.format_value(field)
 if key_prefix is None:
 key_prefix = "/data/attributes"
 pointer = f"{key_prefix}/{field}"
 if isinstance(error, dict):
 errors = []
 for nested_field, nested_error in error.items():
 errors.extend(
 _handle_error_value(response, exc, nested_field, nested_error, pointer)
 )
 return errors
 if isinstance(exc, Http404) and isinstance(error, str):
 # 404 errors don't have a pointer
 return [utils.format_error_object(error, None, response)]
 if isinstance(error, str):
 classes = inspect.getmembers(exceptions, inspect.isclass)
 # DRF sets the `field` to 'detail' for its own exceptions
 if isinstance(exc, tuple(x[1] for x in classes)):
 pointer = "/data"
 return [utils.format_error_object(error, pointer, response)]
 if isinstance(error, list):
 return [
 utils.format_error_object(message, pointer, response) for message in error
 ]
 return [utils.format_error_object(error, pointer, response)]
def exception_handler(exc, context):
 """Return errors from DJA views formatted as per JSON:API spec."""
 # Import this here to avoid potential edge-case circular imports, which
 # crashes with:
 # "ImportError: Could not import 'rest_framework_json_api.parsers.JSONParser' for API setting
 # 'DEFAULT_PARSER_CLASSES'. ImportError: cannot import name 'exceptions'.'"
 #
 # Also see: https://github.com/django-json-api/django-rest-framework-json-api/issues/158
 from rest_framework.views import exception_handler as drf_exception_handler
 # Render exception with DRF
 response = drf_exception_handler(exc, context)
 if not response:
 return response
 # Use regular DRF format if not rendered by DRF JSON API and not uniform
 is_json_api_view = rendered_with_json_api(context["view"])
 is_uniform = json_api_settings.UNIFORM_EXCEPTIONS
 if not is_json_api_view and not is_uniform:
 return response
 # Convert to DRF JSON API error format
 response = format_drf_errors(response, context, exc)
 # Add top-level 'errors' object when not rendered by DRF JSON API
 if not is_json_api_view:
 response.data = utils.format_errors(response.data)
 return response

I'm not the biggest fan of my implementation being recursive - I'm sure it could be re-factored to be iterative. I haven't had the time to refactor due to other commitments.

Also, unfortunately, the implementation of the ListSerializer (when using nested serializers with many=True) does not supply the index of the item that caused the error. There is not much we can do about this, short of changing its to_internal_value implementation to match that of the ListField. (Which I have done for my project.)

Copy link
Member

sliverc commented Aug 18, 2020

@sapiosexual Any progress on this?

This has been hanging for a bit longer than I hoped for... 😄 I would love to release but I have a feeling without proper support of error handling I would need to temporarily remove the nested serializer support from master.

Also when thinking about issue #796 I actually noticed that this issue affects the nested serializer support. As with nested serializer support the nested attribute is not a value in terms of DRF and could be properly formatted with the field names setting. Issue with this is if we fix this later it won't be backwards compatible. So I think before releasing this needs to be addressed as well (or discussed if opinions differ 😉).

I don't have a lot of time at hand to work on those issues myself. Shall we temporarily remove the support for nested serializers or do you think you would find time to work on this?

Thanks for your feedback.

Copy link
Member

sliverc commented Aug 24, 2020

I have found some time to look into this. Extracted some code bits from here and adjusted it. See #815 Please comment in that PR if I have missed out on anything.

Closing this PR in favor of #815

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@auvipy auvipy auvipy requested changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /