Silent skipping of read-only fields that throw exceptions: weird! unintentional?

[ezheidtmann]

I have a serializer with some read-only fields which are populated by a method on the model. This worked great. Then I made a change elsewhere that broke my method. Now my fields don't show up. Instead, I would have expected the exception to reach the browser somehow -- a 500 error from DRF or from Django core.

I discovered that DRF is swallowing the exception thrown inside my function (an AttributeError) and silently skipping the field. This is confusing to me and I don't understand how this is useful; in fact, it seems that the logic is appropriate for a de-serialization (i.e. create or update) but not for output.

If the current behavior is desired, what's the reasoning? And if so, can we add something to the guide about it, under exception handling?

For context, here's my serializer:

class SectionSerializer(serializers.HyperlinkedModelSerializer):
    route_count = serializers.IntegerField(source='get_route_count', read_only=True)

And here's the method on my model:

class Section(Versionable):
    ...
    def get_route_count(self):
        if self.route_type == Section.RT_BOULDER:
            return self.boulderroute_set.count()
        elif self.route_type == Section.RT_SPORT:
            return self.sportroute_set.count()

(My function get_route_count is broken because I removed the .RT_* constants in a refactor)

And this is where my AttributeError is caught in fields.py:

class Field(object):
    ...
    def get_attribute(self, instance):
        """
        Given the *outgoing* object instance, return the primitive value
        that should be used for this field.
        """
        try:
            return get_attribute(instance, self.source_attrs)
        except (KeyError, AttributeError) as exc:
            if not self.required and self.default is empty:
                raise SkipField()
            msg = (
                'Got {exc_type} when attempting to get a value for field '
                '`{field}` on serializer `{serializer}`.\nThe serializer '
                'field might be named incorrectly and not match '
                'any attribute or key on the `{instance}` instance.\n'
                'Original exception text was: {exc}.'.format(
                    exc_type=type(exc).__name__,
                    field=self.field_name,
                    serializer=self.parent.__class__.__name__,
                    instance=instance.__class__.__name__,
                    exc=exc
                )
            )
            raise type(exc)(msg)

Thanks all!

[tomchristie]

Not much we can do about this. We need to catch AttributeError for the case where the field simply maps to an attribute that doesn't exist on the object. If an AttributeError is being raised as part of accessing the field, then we don't have any way to differentiate between that and the attribute itself being missing. If anyone has any carzy bright ideas on how we could inspect the stack to deal with that then we can re-review this, but otherwise there's not anything obvious we can do about this.

[ezheidtmann]

OK, I can imagine a use case where you don't want to fail just because an attribute is missing from the instance. I will see if I can come up with a method to distinguish function failures from missing attributes, as you describe.

Alternatively, would you consider a PR that makes it possible to require a read-only field for serialization? Currently I can't do this because setting required=True, read_only=True raises an exception.

[tomchristie]

would you consider a PR that makes it possible to require a read-only field for serialization?

Not sure without further thought/discussion.