Opened 14 years ago

Closed 11 years ago

#13400 closed Bug (duplicate)

model.DoesNotExist exception raised by QuerySet.get() should include the same nice description of parameters that the MultipleObjectsReturned exception does.

Reported by: aaron Owned by: trentm
Component: Database layer (models, ORM) Version:
Severity: Normal Keywords: model.DoesNotExist, ObjectDoesNotExist, QuerySet.get error message kwargs parameters
Cc: asokoloski@…, Anders Hovmöller Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The current message contained in a DoesNotExist (ObjectDoesNotExist) exception raised by QuerySet.get(...) is roughly:
"%s matching query does not exist." % MODEL_NAME

But the MultipleObjectsReturned exception raised by the same function looks like:
"get() returned more than one %s -- it returned %s! Lookup parameters were %s" % (MODEL_NAME, NUMBER_OF_OBJECTS, KWARGS)

with KWARGS being the keyword arguments passed to the get(...) method. This is great for investigating error message emails sent from a live site -- it makes it really easy to find out which object you were trying to get, and how to fix the root cause.

I suggest that the DoesNotExist error message would benefit from the same descriptiveness. I am attaching a patch that adds this information to the model.DoesNotExist exception.

Attachments (3)

13400-better-doesnotexist-error-message.diff (835 bytes ) - added by aaron 14 years ago.
Patch to give better error message when QuerySet.get(...) raises model.DoesNotExist
13400-better-doesnotexist-error-message-tests-fixed.diff (6.4 KB ) - added by aaron 14 years ago.
Same as previous patch, but with tests fixed.
13400-alt-doesnotexist-error-message.diff (10.9 KB ) - added by trentm 14 years ago.
My alternative improved DoesNotExist error message patch (with updates for tests)

Download all attachments as: .zip

Change History (19)

by aaron, 14 years ago

Patch to give better error message when QuerySet.get(...) raises model.DoesNotExist

comment:1 by aaron, 14 years ago

Has patch: set

comment:2 by Russell Keith-Magee, 14 years ago

Needs tests: set
Triage Stage: UnreviewedAccepted

A reasonable suggestion. However the patch needs work - in particular, this change will break several existing doctests that are relying on the current wording of the error.

by aaron, 14 years ago

Same as previous patch, but with tests fixed.

comment:3 by trentm@…, 14 years ago

The format of the message I came up with was, e.g.:

DoesNotExist: User matching query (id__exact=2L) does not exist.

The patch for that (without the patch for the test cases) is:

Index: django/db/models/query.py
===================================================================
--- django/db/models/query.py   (revision 13020)
+++ django/db/models/query.py   (working copy)
@@ -301,8 +301,9 @@
         if num == 1:
             return clone._result_cache[0]
         if not num:
-            raise self.model.DoesNotExist("%s matching query does not exist."
-                    % self.model._meta.object_name)
+            q = ', '.join([repr(a) for a in args] + ["%s=%r" % i for i in kwargs.items()])
+            raise self.model.DoesNotExist(u"%s matching query (`%s`) does not exist."
+                    % (self.model._meta.object_name, q))
         raise self.model.MultipleObjectsReturned("get() returned more than one %s -- it returned %s! Lookup parameters were %s"
                 % (self.model._meta.object_name, num, kwargs))

If others preferred that wording, then I could make a patch along with the test case updates.

Pros:

  • My patch includes the "args" in the formatting, tho I don't really know if the 'args' are ever non-empty

Cons:

  • Construction of the error string in my patch is slightly more computationally expensive.

comment:4 by trentm@…, 14 years ago

That example string in last message should be (wiki formatting error of mine):

DoesNotExist: User matching query (`id__exact=2L`) does not exist.

comment:5 by aaron, 14 years ago

Sounds good to me, trentm.

comment:6 by aaron, 14 years ago

Cc: asokoloski@… added

by trentm, 14 years ago

My alternative improved DoesNotExist error message patch (with updates for tests)

comment:7 by trentm, 14 years ago

Needs documentation: set
Needs tests: unset

Note to self: there are a couple examples of this traceback in the docs that should probably be added to the patch.

comment:8 by aaron, 14 years ago

Owner: changed from aaron to trentm

Trentm, I'm going to assign this to you now since you're the one working on it. Thanks.

comment:9 by Adam Nelson, 14 years ago

Patch needs improvement: set

Noting that patch needs improvement, as per Trentm

comment:10 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:11 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:12 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:13 by Anders Hovmöller, 12 years ago

Is anyone gonna integrate this? Such a simple fix that improves usability so much seems like a no brainer to me.

comment:14 by Anders Hovmöller, 12 years ago

Cc: Anders Hovmöller added

comment:15 by Russell Keith-Magee, 12 years ago

@boxed: Yes, the idea is a no brainer. That's why the ticket was accepted. However, the fact that the idea is a no-brainer doesn't change the fact that the ticket is marked as needing improvement. That means we need someone to fix the patch, and we need someone else to review that updated patch and mark the ticket as ready for checkin.

You don't need to have special permission to do either of these things -- we just need a cleanly applying patch, addressing all the issues raised, that has been vetted by two people independently. Once it's marked ready for checkin, the core team will commit it (or bump it back to needs improvement, with more comments).

comment:16 by Baptiste Mispelon, 11 years ago

Resolution: duplicate
Status: newclosed

This was a duplicate of #10494 which was fixed in d5b93d3281fe93cbef5de84a52001f64dbd839bd

Note: See TracTickets for help on using tickets.
Back to Top