globalsign / mgo

The MongoDB driver for Go
Other
1.97k stars 230 forks source link

Don't panic when an index with a bad value is found in Indexes(). #298

Closed johnnason closed 5 years ago

johnnason commented 5 years ago

Instead, return an error and let the consumer decide how to proceed.

In practice, I've come across a fair amount of DBs in the wild that exhibit this problem. To replicate locally, I had to do a mongodump, edit the index metadata, and do a mongorestore into a 2.6.X DB. More modern versions of mongo prevent it from happening, but the reality is there's junk in the wild.

Panicing in this case makes us refactor consumer code to defer and recover, which could just be an error check instead.

eminano commented 5 years ago

Thanks a lot for this PR @johnnason!

You are right, this is the right approach instead of forcing to recover from panics. However these are breaking changes for users that might have specific behaviour within the panic handling.

I think the benefits from this PR outweigh the few cases where this is going to be a problem, so I'd be happy to merge it. @domodwyer, what do you think?

domodwyer commented 5 years ago

Hey @johnnason

As @eminano says, this is a breaking change but I would imagine the breakage will be slim-to-none given real world usage - it's a great change, thanks for taking the time!

Dom