sapics/geoip-country

Improvements to updating MaxMind databases

Closed this issue · 1 comments

Hello!

We have started using your package recently and to satisfy the MaxMind license condition to keep databases up-to-date, we had to implement some sophisticated runtime logic.
In the current geoip-country implementation - there is a dedicated updatedb script that can be used. But because this is just a CLI script - using it from the code is not a very friendly experience.
For example, let's say there is a service that is running for a long period of time without restarts. To keep the geo databases up to date - there is a need to update them during runtime from time to time.

With the current update logic implementation, one has to add a script to the service package.json. e.g.

"util:update-geo-db": "cd node_modules/geoip-country && npm run-script updatedb"

And then use child_process exec function to call that script from the service code to initiate service update. Using something like import('geoip-country/scripts/updatedb') from the code to initiate db update is not an option, since both success and error cases execute process.exit, stopping the service. So child_process is used to be able to call the cli script, catch errors and properly process them. And to process them, one has to look into the full stdout output. Current output looks like this, and the message Successfully Updated Databases from MaxMind is not printed (at least if this is executed using child_process):

'$ cd node_modules/geoip-country && npm run-script updatedb\n' +
'\n' +
'> geoip-country@4.0.95 updatedb E:\\Projects\\NavyCMS\\CustomizableSolution\\node_modules\\geoip-country\n' +
'> node scripts/updatedb.js\n' +
'\n' +
'Fetching edition GeoLite2-Country-CSV from https://geoip.maxmind.com/app/geoip_download\n' +
'Retrieving GeoLite2-Country-CSV.zip ... DONE\n' +
'Extracting GeoLite2-Country-CSV.zip ...Processing Lookup Data (may take a moment) ... DONE\n' +
'Processing Data (may take a moment) ...\n' +
'Still working (90489) ...\n' +
'Still working (193209) ...\n' +
'Still working (291078) ... DONE\n' +
'Processing Data (may take a moment) ...\n' +
'Still working (71060) ...\n' +
'Still working (140361) ...\n' +
'Still working (211170) ...\n' +
'Still working (283102) ...\n' +
'Still working (357311) ... DONE\n'

So the best way I currently found to determine that script succeeded is to make sure that stdout prints DONE exactly 4 times and stderr is empty. (Because a script can also fail because of incorrect license key and produce stdout output without stderr - I must dig into stdout text to determine if the update was a success or not)

In addition to this, if service is a part of monorepo, this can become a bit more complicated, because, in monorepo, geoip-country package can be located in node_modules at the root of the monorepo, but when service is deployed individually - package will be located in the root of the service. This makes it necessary to have 2 scripts and to perform a call to one of them based on whether the service is launched during development or in a deployed environment. e.g.

"util:update-geo-db-dev": "cd ../../../node_modules/geoip-country && npm run-script updatedb",
"util:update-geo-db-prod": "cd node_modules/geoip-country && npm run-script updatedb"

All of these inconveniences can be easily solved if geoip-country lib would export a dedicated function that performs an update of the databases.
This new function can then be used by updatedb script and it would be usable from the code for the consumers of the geoip-country lib. As one possible implementation, the function would be async, would receive an optional license key parameter (since it can also be taken from env variable), would return void and throw errors with corresponding messages if the update fails for some reason (and would not perform process.exit). As an extra, custom errors with code values could be thrown, so that calling code could easily distinguish the reason using them to provide more fitting error messages for the specific usage cases (in case original error messages are not too user friendly).

An extra extra nice-to-have feature would be to track db version and not perform db update if current local db files already have the latest version (but I'm not sure if there is an easy way to do so) :)

This solution would be a lot cleaner and allow a more convenient development experience.
I hope these suggestions can be taken into account and hopefully implemented.

Thank you! :)

Thank you @PizzaPartyInc for improve suggestions!
Sorry for some inconveniences to updating the database, and late for response.

1: Fixed the updatedb.js to print Successfully Updated Databases from MaxMind in v4.0.119.
2: Added new method updateDatabase([license_key], [callback]) in v4.1.0.

Tested successfully on node16 in windows 10.
If anyone has had success on linux or macos, we would appreciate it if you could report it here.