projectblacklight/blacklight — pull request #662 by jrochkind (2013-12-23 18:45:28+00:00)
Title: rescue ActiveRecord::RecordNotFound, not everything

The one-line rescue found here ends up rescueing all exceptions. I think that was making troubleshooting of #646 a lot harder. It seems probably seldom appropriate to rescue all exceptions (which means the one-line rescue is probably seldom appropriate, sadly).

I think this was meant to mainly rescue ` ActiveRecord::RecordNotFound` -- if there's a search id in params or session that can't be found in the database, don't complain, just say no search context found.

So I changed it to do so in this pull request.

One reason I PR'd this for code review instead of just commiting it is -- I have not changed or added any tests. There did not seem to be any existing tests that touched this func at all (removing the rescue altogether didn't seem to make any tests fail, neither does this PR). It seems like tests might be a good idea -- but I'm having trouble figuring out how to do appropriate tests.

So I guess I'm asking for help, either someone else saying "Yeah, tests would be nice but it's okay to commit this without them," or someone else helping with the tests, by writing some or giving me some guidance. Thanks!