marshmallow icon indicating copy to clipboard operation
marshmallow copied to clipboard

Marshmallow hides errors from broken properties

Open symstu opened this issue 3 years ago • 1 comments

Hi, here is my sample:

from dataclasses import dataclass
from marshmallow import Schema, fields


class TestSchema(Schema):
    id = fields.Integer()
    name = fields.String()


@dataclass
class Data:
    id: int

    @property
    def name(self):
        data = []
        data.get('hello')

print(TestSchema().dump(Data(id=1)))

In this case i've expected to see error

Traceback (most recent call last):
  File "test_mash.py", line 19, in <module>
    Data(id=1).name
  File "test_mash.py", line 17, in name
    data.get('hello')
AttributeError: 'list' object has no attribute 'get'

But, instead of this marshmallow returns nothing. I've discovered, that i have option to set default value for attribute "name" and maybe for some cases that's ok. In my real project this property is more complicated and can not have default value. I expect to see errors.

My propose is to set just an a logger to detect such cases. I think, unhiding here errors may become huge changes and broke many apps.

Here is my PR: https://github.com/marshmallow-code/marshmallow/pull/2002

symstu avatar Jun 15 '22 13:06 symstu

Changing AttributeError to other exception classes does not reproduce the issue. getattr uses the default when an AttributeError is raised in a property method.

class Data:
    @property
    def name(self):
        [].x

getattr(Data(), 'name', 'default')
# 'default'

[hasattr] is implemented by calling getattr(object, name) and seeing whether it raises an AttributeError or not.

https://docs.python.org/3/library/functions.html#hasattr

I'm not convinced that marshmallow should contain logging for this. This could happen anywhere outside of marshmallow and would be equally frustrating to debug. This is related to https://github.com/python/cpython/issues/90143 and https://github.com/python/cpython/issues/81560, appears to be a long standing known issue, and I don't anticipate any progress on the issue within the decade.

If you are determined to avoid these silent errors in the future I would recommend patching property() like https://github.com/python/cpython/issues/90143#issuecomment-1093939465.

class DynamicAttributeError(Exception):
    pass

class UnexpectedAttributeError(Exception):
    pass

def safe_property(getter, *args, **kwargs):
    def _getter(*args, **kwargs):
        try:
            return getter(*args, **kwargs)
        except AttributeError:
            raise UnexpectedAttributeError()
        except DynamicAttributeError:
            raise AttributeError()
    return property(_getter, *args, **kwargs)
class ComplexData:
    @safe_property
    def name(self):
        raise DynamicAttributeError()

getattr(ComplexData(), 'name', 'default')
# default
class SafeData:
    @safe_property
    def name(self):
        [].x

getattr(SafeData(), 'name', 'default')
Traceback (most recent call last):
  File "<stdin>", line 4, in _getter
  File "<stdin>", line 4, in name
AttributeError: 'list' object has no attribute 'x'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 6, in _getter
__main__.UnexpectedAttributeError

deckar01 avatar Jun 15 '22 15:06 deckar01

Thanks for bringing this up.

I'm closing this as not marshmallow-specific.

This could happen anywhere outside of marshmallow and would be equally frustrating to debug. This is related to https://github.com/python/cpython/issues/90143 and https://github.com/python/cpython/issues/81560, appears to be a long standing known issue, and I don't anticipate any progress on the issue within the decade.

Indeed I don't think this deserves logging.

lafrech avatar Oct 17 '22 19:10 lafrech