msimerson / maxmind-geolite-mirror

Mirror MaxMind GeoIP dbs from geolite.maxmind.com
https://www.npmjs.com/package/maxmind-geolite-mirror
MIT License
10 stars 5 forks source link

don't silently fail on the first 404 #23

Closed tcql closed 5 years ago

tcql commented 5 years ago

currently if one download 404's (as GeoIPASNumv6.dat does), no further databases are downloaded and the application silently fails.

msimerson commented 5 years ago

There's a few issues:

msimerson commented 5 years ago

@tcql , I created #24 that takes this idea and goes a bit further with it. It passes back the error via the callback (and does similarly in two other places instead of throw) and adds an error handler in the caller to log and exit upon error.

Exiting after encountering an error is by design. When cases (like 404) arise where it makes sense to handle the error, code was and will be added to so something sensible for that particular error condition.

Thoughts?

tcql commented 5 years ago

if its exiting silently for you, this does nothing for that

so the "silent" part is that there's no explanation as to why it stops. maybe poorly worded as a description. If a 404 or other error happens, that is logged already (so, not silent, 🎉), but not continuing to download is not indicated.

this continues after encountering any error (including a 403 forbidden). Which isn't desirable.

I think I wasn't understanding the reasoning here / why it was desirable to give up when any error occurs. Especially something like a 404 which is out of control of users to do anything about. Maybe either a way to pass in the names of which products you want to download, or a way to select whether you want fail immediately or not, would be useful 🤔

the 404 issue was already fixed in #22

There's a new 404, I guess? GeoIPAsNumv6.dat fails. Should that be removed / replaced? Looking around at maxmind's database docs they don't seem to even advertise the .dat products anywhere and are pushing either .mmdb or .csv

24 looks good, much more sensible in terms of error handling! thanks :)