FinalsClub / karmaworld

KarmaNotes.org v3.0
GNU Affero General Public License v3.0
7 stars 6 forks source link

IndexDen conditionally traps errors #281

Closed btbonval closed 10 years ago

btbonval commented 10 years ago

compare with try/except: https://github.com/FinalsClub/karmaworld/blob/2ee0c69efffdca3e43e12f0256ae144c6d383807/karmaworld/apps/notes/models.py#L288-L296

same file, just a bit lower, no try/except: https://github.com/FinalsClub/karmaworld/blob/2ee0c69efffdca3e43e12f0256ae144c6d383807/karmaworld/apps/notes/models.py#L310-L311

btbonval commented 10 years ago

Actually, the SearchIndex class looks like it might be a better place to try/except these sorts of errors instead of all the places it can be called? SearchIndex isn't explicitly throwing the exceptions that occur, so I assume that object isn't trying to throw useful information to a higher level.

charlesconnell commented 10 years ago

We can catch errors and log them in SearchIndex.add_note() and SearchIndex.update_note(). But we should allow errors that occur in SearchIndex.__init__() to be thrown to a higher level, since callers need to know if they've got a valid object. Also, errors in SearchIndex.search() should be thrown to a higher level since users should be notified if their search had an error. So basically, I don't want to change how we're handling errors, but I will but a try block in around the call to SearchIndex.remove_note()

btbonval commented 10 years ago

Fair enough. I thought it'd be easier to put the captures inside SearchIndex, but I think we're solid as long as we're consistent about trapping errors.