Skip to content
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

Method hooks swallow AttributeError #395

Closed
bereal opened this issue Feb 17, 2016 · 5 comments
Closed

Method hooks swallow AttributeError #395

bereal opened this issue Feb 17, 2016 · 5 comments
Milestone

Comments

@bereal
Copy link

bereal commented Feb 17, 2016

If AttributeError is raised by a Method/Function hook, it is silently swallowed:

class C(Schema):
    f = fields.Method('_bad_serialize')
    def _bad_serialize(self, obj):
        return self._do_something_else(obj)  # raises AttributeError

test_type = namedtuple('test_type', 'f')
C(strict=True).dump(test_type(123))  # returns empty list

This makes finding the bugs harder. I understand the rationale and I guess fixing it might make some code backward incompatible, but is there a way to add at least some warning mode?

@hanselpaze
Copy link

This is super annoying... and should at least come with a way to switch off this behaviour for debugging purposes.

@sloria
Copy link
Member

sloria commented May 24, 2016

I intend to look into solutions for this in marshmallow 3.0

@sloria sloria added this to the 3.0 milestone May 24, 2016
@aclowes
Copy link

aclowes commented Jul 15, 2016

I don't understand the original reasons for writing it this way, except that attribute-based field types are omitted if the attribute is missing. Options might be:

  • add validation to serialization, and provide a built-in 'value exists' validator that can be easily used
  • add a new flag that tells the class not to squash the error

@sloria
Copy link
Member

sloria commented Feb 18, 2017

As mentioned in #582 :

Yes, the AttributeError is a wart from marshmallow's early days that accidentally made it into 2.0. I'm leaning towards removing the exception handling in marshmallow 3.0 and cutting an early release.

If there's interest in getting this fixed in 2.0, I can look into merging #582, but I'd rather users just upgrade to 3.0.

@sloria sloria modified the milestones: 3.0, 3.0a Feb 18, 2017
@sloria
Copy link
Member

sloria commented Feb 19, 2017

This is addressed in 54f731d

@sloria sloria closed this as completed Feb 19, 2017
sloria added a commit that referenced this issue Feb 19, 2017
...and fix some errors found by flake8
llazzaro pushed a commit to infobyte/faraday that referenced this issue Jun 28, 2018
It didn't correctly handle values other than a comma-separated string
nor empty strings
A combination of bugs in the tests, the load_references method and
marshmallow 2.13.6 made the test altough the implementation was
incorrect.

See marshmallow-code/marshmallow#395 for more
info on the marshmallow bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants