AssertionError on resolve_serializer

[tfranzel]

I just hit this.

I suggest that assert is never used without a second arg which explains the failed assertion. There are code quality tools which can enforce this. If you want these assertions to cause issues to be raised, it helps to have assertion messages let people raise bugs which can be used to identify the problem.

Since this component is py36+, it could also use should_be instead of assert, which auto-voodoo creates useful assertion error messages.

And as usual, I would prefer these are warnings. In my case it is a custom lazy-ily defined serializer and it could be safely ignored as inspecting it wont work anyway. The following worked for me.

-        assert is_serializer(serializer)
+        if not is_serializer(serializer):
+            return

Originally posted by @jayvdb in #120 (comment)

[tfranzel]

@jayvdb i agree with you on this. the question is where to catch this properly. your fix may hide an underlying issue. i would like to understand what went wrong.

pretty much all resolve_serializer call are already shielded with is_serializer and do graceful handling. if you provide more context we might find a way to be graceful here. i have a hard time seeing how you were able to get there.

in any case there needs to be more info provided to the user. thats for sure.

[jayvdb]

#120 (comment) already says how to reproduce.

[tfranzel]

i get what you used but not how you used it. would you be so kind to narrow it down with a snippet. i'll happily debug it then.

[jayvdb]

I used it just like described in the README.

It was also a serializers.ListSerializer which hit that failure, and now your fix on master has made that go away.

But then there is also
https://github.com/peopledoc/django-formidable/blob/master/formidable/serializers/child_proxy.py which hits another failure.

  File "/usr/lib/python3.8/site-packages/drf_spectacular/generators.py", line 162, in get_schema
    paths=self.parse(request, public),
  File "/usr/lib/python3.8/site-packages/drf_spectacular/generators.py", line 142, in parse
    operation = view.schema.get_operation(path, path_regex, method, self.registry)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 66, in get_operation
    request_body = self._get_request_body()
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 789, in _get_request_body
    component = self.resolve_serializer(serializer, 'request')
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 930, in resolve_serializer
    component.schema = self._map_serializer(serializer, direction)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 585, in _map_serializer
    schema = self._map_basic_serializer(serializer, direction)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 644, in _map_basic_serializer
    schema = self._map_serializer_field(field, direction)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 408, in _map_serializer_field
    schema = self._map_serializer_field(field.child, direction)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 390, in _map_serializer_field
    meta = self._get_serializer_field_meta(field)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 618, in _get_serializer_field_meta
    if field.read_only:
AttributeError: 'LazyChildProxy' object has no attribute 'read_only'

To get it working I did

--- a/drf_spectacular/openapi.py
+++ b/drf_spectacular/openapi.py
@@ -695,18 +695,18 @@ class AutoSchema(ViewInspector):
 
     def _get_serializer_field_meta(self, field):
         meta = {}
-        if field.read_only:
+        if getattr(field, 'read_only', None):
             meta['readOnly'] = True
-        if field.write_only:
+        if getattr(field, 'write_only', None):
             meta['writeOnly'] = True
-        if field.allow_null:
+        if getattr(field, 'allow_null', None):
             meta['nullable'] = True
-        if field.default is not None and field.default != empty and not callable(field.default):
+        if getattr(field, 'default', None) is not None and field.default != empty and not callable(field.default):
             default = field.to_representation(field.default)
             if isinstance(default, set):
                 default = list(default)
             meta['default'] = default
-        if field.help_text:
+        if getattr(field, 'help_text', None):
             meta['description'] = str(field.help_text)
         return meta
 

I dont really want it fixed; it isnt important; there are more important PRs and issues to be solved, including several that were closed without being fixed.

[jayvdb]

It would also be good to have a policy change, to stop using assert like this, maybe converting all of them to should_be - an assert is useless unless it gives sufficient context to explain the unexpected, and most of these eight asserts do not even give the value which was incorrect. Ideally these errors should also explain the probable source of the problem to the user who isnt a developer.

drf_spectacular/extensions.py:        assert self.name, 'name must be specified'
drf_spectacular/openapi.py:        assert isinstance(model_field, models.Field)
drf_spectacular/openapi.py:            assert field.source_attrs, 'ReadOnlyField needs a proper source'
drf_spectacular/openapi.py:                assert False, 'ReadOnlyField target must be property or model field'
drf_spectacular/openapi.py:        assert is_serializer(serializer)
drf_spectacular/plumbing.py:        assert severity in ['warning', 'error']
drf_spectacular/plumbing.py:        assert self.name and self.type and self.object
drf_spectacular/plumbing.py:    assert view.versioning_class

[tfranzel]

robustified _get_serializer_field_meta. only a 95% solution because 100% would have required an interface change on the extensions. we'll do that if somebody actually encounters those 5% in practice.

checked all assert statements. added/changed descriptions where it makes sense. i neglected the descriptions where the assumption is checked before entering. those can only fail if we break the code.