Code

Opened 6 years ago

Closed 6 years ago

#6833 closed (wontfix)

Leaving the u off a string returned from __unicode__ results in horribly hard to debug bugs

Reported by: Simon Willison Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by ramiro)

I hit a nasty bug where weird unicode errors were being thrown in the admin application, which it turned out was caused because I was returning a bytestring from a __unicode__ method. It would be great if Django could spot this mistake and throw a more humanly readable exception. This might just involve a few assertions in the admin code.

Attachments (0)

Change History (3)

comment:1 Changed 6 years ago by ramiro

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by programmerq

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 6 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

I think this is probably a bit optimistically hopeful, unfortunately. We're talking about the __unicode__ method; one of Python's base methods, essentially. Python itself forcibly converts the returned value from that method to have unicode type. So the only way to detect any problems would be to do something convoluted such as having Django's Model.__unicode__ actually call some other Model.not_really_unicode() method and it's that latter method that you implement. That would look really unnatural (even when we choose a less stupid name for the wrapped method), given then number of __unicode__ methods one writes in the course of events.

I think Django code should still always look like Python. You could write your own __unicode__ wrapper as above in a subclass of Model and always inherit from MyModelSubclass, I guess. But I wouldn't want to put that into Django itself, since it's punishing (readability and performance-wise) everybody else's code.

I agree that errors like this are sometimes hard to debug because of the forced type coercion and it would have been better if __unicode__ raised an exception at that point. But that's the way Python is written.

So although I seem to be wontfixing a bunch of your bugs today, Simon (and I'm really not picking on you; it's just the ticket contents, not the reporter. Honest!), I think this is one of those things we can't do much about but where you could program defensively as indicated above if you were willing to write not-quite-Python and pay the readability trade-off.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.