Skip to content

Conversation

@orf
Copy link
Contributor

@orf orf commented Oct 16, 2015

This small pull request includes the original exception traceback in the re-raised TypeError, re: #3503. If there is an error somewhere during the create operation the error message turns from:

...
File "/home/vagrant/venv/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 863, in create
  raise TypeError(msg)

TypeError: Got a `TypeError` when calling `Document.objects.create()`. This may be because you have a writable...

Original exception text was: 'NoneType' is not iterable.

To the much more helpful:

File "/home/vagrant/venv/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 863, in create
    raise TypeError(msg)

TypeError: Got a `TypeError` when calling `Document.objects.create()`.  This may be because you have a writable field on...

Original exception was:
 Traceback (most recent call last):
  File "/home/vagrant/venv/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 846, in create
    instance = ModelClass.objects.create(**validated_data)
  File "/home/vagrant/venv/local/lib/python2.7/site-packages/django/db/models/manager.py", line 127, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/vagrant/venv/local/lib/python2.7/site-packages/django/db/models/query.py", line 348, in create
    obj.save(force_insert=True, using=self.db)
  File "/home/x/document/models.py", line 212, in save
    self.document_template.populate_document(self)
  File "/home/x/document_template/models.py", line 82, in populate_document
    for i in some_method_that_might_return_none():
TypeError: 'NoneType' object is not iterable

@xordoquy
Copy link
Contributor

I wonder if we could take advantage of the nested exceptions with Python 3.x

@orf
Copy link
Contributor Author

orf commented Oct 16, 2015

Yes, my first thought that using raise x from y would be perfect, however the six docs imply that this is doesn't do anything on Py2. I figured including the traceback in the message was a good substitute.

@jpadilla
Copy link
Contributor

There might be an interesting opportunity in improve exception messages like this in some places to include the original. Similar case in #3490.

@lovelydinosaur
Copy link
Contributor

Yup, looks good!

@lovelydinosaur
Copy link
Contributor

Leaving this open until have time to also address #3490, and also review if there are any other similar cases we could be addressing.

Pull requests addressing this throughout from anyone else also most welcome if you happen to get to it before I do.

@lovelydinosaur lovelydinosaur merged commit d540f02 into encode:master Aug 19, 2016
@lovelydinosaur
Copy link
Contributor

On second thoughts may as well merge this now.
Pull requests addressing similar issues elsewhere in the codebase very much welcome.

@orf
Copy link
Contributor Author

orf commented Aug 19, 2016

Woo! Thank you @tomchristie, you don't know how happy this makes me.

@lovelydinosaur
Copy link
Contributor

😄 Most welcome - appreciate the input!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants