diff --git a/src/lib/geoip/client.ts b/src/lib/geoip/client.ts index 3c862fbe..27410913 100644 --- a/src/lib/geoip/client.ts +++ b/src/lib/geoip/client.ts @@ -1,6 +1,5 @@ import _ from 'lodash'; import config from 'config'; -import type { Logger } from 'winston'; import newrelic from 'newrelic'; import type { CacheInterface } from '../cache/cache-interface.js'; import { InternalError } from '../internal-error.js'; @@ -27,16 +26,12 @@ export type NetworkInfo = { asn: number; }; -export const createGeoipClient = (): GeoipClient => new GeoipClient( - new RedisCache(getRedisClient()), - scopedLogger('geoip'), -); +const logger = scopedLogger('geoip'); + +export const createGeoipClient = (): GeoipClient => new GeoipClient(new RedisCache(getRedisClient())); export default class GeoipClient { - constructor ( - private readonly cache: CacheInterface, - private readonly logger: Logger, - ) {} + constructor (private readonly cache: CacheInterface) {} async lookup (addr: string): Promise { const results = await Promise @@ -155,7 +150,7 @@ export default class GeoipClient { } if (!best || best.provider === 'fastly') { - this.logger.error(`failed to find a correct value for a field "${field}"`, { field, sources }); + logger.error(`failed to find a correct value for a field "${field}"`, { field, sources }); throw new Error(`failed to find a correct value for a field "${field}"`); } @@ -163,7 +158,10 @@ export default class GeoipClient { } public async lookupWithCache (key: string, fn: () => Promise): Promise { - const cached = await this.cache.get(key); + const cached = await this.cache.get(key).catch((error: Error) => { + logger.error('Failed to get cached geoip info for probe.', error); + newrelic.noticeError(error, { key }); + }); if (cached) { return cached; @@ -173,7 +171,7 @@ export default class GeoipClient { const ttl = Number(config.get('geoip.cache.ttl')); await this.cache.set(key, info, ttl).catch((error: Error) => { - this.logger.error('Failed to cache geoip info for probe.', error); + logger.error('Failed to cache geoip info for probe.', error); newrelic.noticeError(error, { key, ttl }); }); diff --git a/test/tests/unit/geoip.test.ts b/test/tests/unit/geoip.test.ts index 1aa72073..06b8c1d9 100644 --- a/test/tests/unit/geoip.test.ts +++ b/test/tests/unit/geoip.test.ts @@ -6,8 +6,8 @@ import type { LocationInfo } from '../../../src/lib/geoip/client.js'; import { fastlyLookup } from '../../../src/lib/geoip/providers/fastly.js'; import GeoipClient from '../../../src/lib/geoip/client.js'; import NullCache from '../../../src/lib/cache/null-cache.js'; -import { scopedLogger } from '../../../src/lib/logger.js'; import { populateMemList } from '../../../src/lib/geoip/whitelist.js'; +import type { CacheInterface } from '../../../src/lib/cache/cache-interface.js'; const mocks = JSON.parse(fs.readFileSync('./test/mocks/nock-geoip.json').toString()) as Record; @@ -19,16 +19,61 @@ describe('geoip service', () => { before(async () => { await populateMemList(); - client = new GeoipClient( - new NullCache(), - scopedLogger('geoip:test'), - ); + client = new GeoipClient(new NullCache()); }); afterEach(() => { nock.cleanAll(); }); + describe('GeoipClient', () => { + it('should work even if cache is failing', async () => { + class BrokenCache implements CacheInterface { + async delete (): Promise { + throw new Error('delete is broken'); + } + + async get (): Promise { + throw new Error('get is broken'); + } + + async set (): Promise { + throw new Error('set is broken'); + } + } + + const clientWithBrokenCache = new GeoipClient(new BrokenCache()); + nock('https://globalping-geoip.global.ssl.fastly.net') + .get(`/${MOCK_IP}`) + .reply(200, mocks['00.00'].fastly); + + nock('https://ipinfo.io') + .get(`/${MOCK_IP}`) + .reply(200, mocks['00.00'].ipinfo); + + nock('https://geoip.maxmind.com/geoip/v2.1/city/') + .get(`/${MOCK_IP}`) + .reply(200, mocks['00.00'].maxmind); + + const info = await clientWithBrokenCache.lookup(MOCK_IP); + + expect(info).to.deep.equal({ + continent: 'SA', + country: 'AR', + normalizedRegion: 'south america', + region: 'South America', + state: undefined, + city: 'Buenos Aires', + normalizedCity: 'buenos aires', + asn: 61_493, + latitude: -34.602, + longitude: -58.384, + network: 'interbs s.r.l.', + normalizedNetwork: 'interbs s.r.l.', + }); + }); + }); + it('should use maxmind & digitalelement consensus', async () => { nock('https://globalping-geoip.global.ssl.fastly.net') .get(`/${MOCK_IP}`)