search

oschwald / maxminddb-golang

MaxMind DB Reader for Go
ISC License
576 stars 99 forks source link

Return err when lookup fails. #52

Closed rowland closed 4 years ago

rowland commented 5 years ago

One would hope to be able to distinguish between an IP address with no registered country (or city) without examining each and every member of the returned struct, but that does not appear to be possible, because maxminddb.Reader.Lookup obscures the difference.

This change breaks many tests that currently expect nil, even though a record was not found.

If it is desired that errors be reserved for operational issues, this information needs to be made available to geoip2 from maxminddb so that geoip2 can return a nil for *City or *Country instead of an error.

It would be possible to work around this issue without breaking the existing API by using LookupOffset and Decode in maxminddb, but the maxminddb.Reader instance is private in geoip2, so 3rd-party code can't reach it.

Alternatively, in order to maintain strict backwards-compatibility, I could implement new methods in geoip2.Reader that would lookup records and behave as desired, but that would mean adding 7 new methods or a new mode for the 7 existing methods.

oschwald commented 5 years ago

This is a dupe of #16. I am not looking to do a breaking release at this time.

Generally, I would recommend checking whether the data you are interested in is present rather than relying on whether the lookup succeeded. Even if the lookup succeeds, it doesn't mean that any particular piece of data will be present in the result.

oschwald commented 4 years ago

59 addresses this issue in a different way with the new LookupNetwork method. Closing in favor of that.