Merge branch 'master' into new-providers

jsdelivr · Jul 12, 2023 · 35a6ed9 · 35a6ed9
2 parents 21a50aa + 99882e9
commit 35a6ed9
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 19 deletions.
diff --git 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';
@@ -30,16 +29,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<ProbeLocation> {
 		const results = await Promise
@@ -156,7 +151,7 @@ export default class GeoipClient {
 		const best = ranked[0];
 
 		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}"`);
 		}
 
@@ -165,7 +160,10 @@ export default class GeoipClient {
 	}
 
 	public async lookupWithCache<T> (key: string, fn: () => Promise<T>): Promise<T> {
-		const cached = await this.cache.get<T>(key);
+		const cached = await this.cache.get<T>(key).catch((error: Error) => {
+			logger.error('Failed to get cached geoip info for probe.', error);
+			newrelic.noticeError(error, { key });
+		});
 
 		if (cached) {
 			return cached;
@@ -175,7 +173,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
@@ -5,10 +5,9 @@ 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 nockGeoIpProviders from '../../utils/nock-geo-ip.js';
-import { geoIpMocks } from '../../utils/nock-geo-ip.js';
+import nockGeoIpProviders, { geoIpMocks } from '../../utils/nock-geo-ip.js';
+import { CacheInterface } from '../../../src/lib/cache/cache-interface.js';
 
 const MOCK_IP = '131.255.7.26';
 
@@ -18,16 +17,52 @@ 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<undefined> {
+					throw new Error('delete is broken');
+				}
+
+				async get (): Promise<undefined> {
+					throw new Error('get is broken');
+				}
+
+				async set (): Promise<void> {
+					throw new Error('set is broken');
+				}
+			}
+
+			const clientWithBrokenCache = new GeoipClient(new BrokenCache());
+			nockGeoIpProviders();
+
+			const info = await clientWithBrokenCache.lookup(MOCK_IP);
+
+			expect(info).to.deep.equal({
+				continent: 'NA',
+				country: 'US',
+				state: 'TX',
+				city: 'Dallas',
+				region: 'Northern America',
+				normalizedRegion: 'northern america',
+				normalizedCity: 'dallas',
+				asn: 20001,
+				latitude: 32.001,
+				longitude: -96.001,
+				network: 'The Constant Company LLC',
+				normalizedNetwork: 'the constant company llc',
+			});
+		});
+	});
+
 	it('should choose top prioritized provider from the winners majority', async () => {
 		nockGeoIpProviders({ ipmap: 'default', ip2location: 'default', maxmind: 'argentina', fastly: 'argentina', ipinfo: 'argentina' });