[SERVER-1572] If there are no matches, findandmodify() should return an empty result, not raise an exception. Created: 05/Aug/10  Updated: 19/May/14  Resolved: 25/Oct/10

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: 1.5.6
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Nick Leippe Assignee: Mathias Stearn
Resolution: Won't Fix Votes: 3
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Ubuntu 10.4, x86-64


Issue Links:
Related
is related to PYTHON-170 add findAndModify helper method Closed
Operating System: ALL
Participants:

 Description   

See comments on (closed) PYTHON-152

findandmodify() raises pymongo.errors.OperationFailure when there are no matches:

File "build/bdist.linux-x86_64/egg/pymongo/database.py", line 294, in command
pymongo.errors.OperationFailure: command SON([('findandmodify', 'my_queue'), ('sort',

{'_id': -1}

), ('query', {'started': {'$exists': False}}), ('update', {'$set': {'started': 1280965374}})]) failed: No matching object found

According to the mongodb documentation for findandmodify:

"This command can be used to atomically modify a document (at most one) and return it."

This implies that findandmodify() behaves much like the find() command--it is perfectly acceptable to not have a match at all.
Such behavior makes perfect sense. I do not believe it should constitute a command failure simply because there are no matches.
No matches should return an empty result, not raise a command failure exception.

IMO, command failure, should be when it finds a match but fails to make the modification, or some other actual but unexpected condition occurs.

This is a very common use case--a consumer pulling things off a queue. When the queue is empty, it shouldn't be generating an exception, just returning empty. It yields much nicer code.



 Comments   
Comment by Mathias Stearn [ 25/Oct/10 ]

I think the server behavior is correct since find_and_modify on a non-existent object with upsert=false is usually an error and at the very least should be handled differently from normal behavior since no modification has taken place.

I helped your issue by adding a helper in python that matches the shell's behavior. That is, returning null/None in that case and {} as the original object when inserting on an upsert.

Comment by Mathias Stearn [ 22/Oct/10 ]

I think I will just add a find_and_modify helper to collections in pymongo. That should take care of this.

Comment by Nick Leippe [ 05/Aug/10 ]

My last comment turns out is only half correct.

The BaseException still has an args attribute that can be analyzed to implement the work around.
But, to do so requires: test the length (since it's optional), test the type (make sure it's a string or unicode type), then finally check the contents. A lot of work for a common, usual case.

So, imo, the two solutions are:
1) return empty
2) create a specific exception for this case

Creating a specific exception for a single case screams that it shouldn't be an exception. Hitting eof on a fd doesn't raise--you expect to hit it eventually, just like in this case, you expect to exhaust a queue at some point.

Comment by Nick Leippe [ 05/Aug/10 ]

In attempting to work around this, I've discovered that this behavior is even more critical to fix.
Here's why:

It raises pymongo.errors.OperationFailure, which is derived from BaseException. There are possibly many reasons why this command could legitimately fail and would need to raise this exception. Therefore, in order to ascertain that it is the legitimate reason of not finding a match, the exception needs to be examined.

The only data in the exception is the message attribute string which can be tested for the presence of the suffix "failed: No matching object found". Good and dandy right?

Except that when doing so yields:

DeprecationWarning: BaseException.message has been deprecated as of Python 2.6

Thus, when that attribute is gone, there will be no means of distinguishing this reason for this exception being raised from any other. This work-around will be broken as soon as the BaseException.message attribute is removed. (3.0? I couldn't find anywhere saying when that will be done.)

Comment by Nick Leippe [ 05/Aug/10 ]

If there's concern about breaking existing code via a change in behavior, I suggest adding a new, optional parameter, which defaults to the existing behavior such as "required" or "must_exist", or something along those lines.

Generated at Thu Feb 08 02:57:24 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.