From 5f142dfa2a7fe1988a643b42666acab9f5cdb9b6 Mon Sep 17 00:00:00 2001
From: Jun <cygoh@umich.edu>
Date: Sat, 24 Feb 2018 21:13:37 -0800
Subject: [PATCH] fix(geolocate): return error where there is no ip info

---
 src/geolocate.js              | 18 +++++++++++-------
 src/utils/constants.js        |  3 +++
 tests/amazonGeotarget.test.js | 35 ++++++++++++++++++-----------------
 3 files changed, 32 insertions(+), 24 deletions(-)
 create mode 100644 src/utils/constants.js

diff --git a/src/geolocate.js b/src/geolocate.js
index 4c27eb8..b399f58 100644
--- a/src/geolocate.js
+++ b/src/geolocate.js
@@ -1,13 +1,17 @@
+import noIpInfoErrorMsg from './utils/constants';
+
 export default class GeolocateService {
-  static async geolocateFreeGeoIp() {
-    const response = await fetch('https://freegeoip.net/json/');
-    const ipInfo = await response.json();
+  static async geolocateFreeGeoIp(ip) {
+    const ipQuery = ip || '';
+    const response = await fetch(`https://freegeoip.net/json/${ipQuery}`);
+    const ipInfo = await response.json().catch(() => new Error(noIpInfoErrorMsg));
     return ipInfo;
   }
 
-  static async geolocateIPAPI() {
-    const response = await fetch('https://ipapi.co/country');
-    const countryCode = await response.text();
-    return countryCode;
+  static async geolocateIPAPI(ip) {
+    const ipQuery = ip ? `${ip}/` : '';
+    const response = await fetch(`https://ipapi.co/${ipQuery}country`);
+    const countryCode = await response.text().catch(() => new Error(noIpInfoErrorMsg));
+    return countryCode !== 'Undefined' ? countryCode : new Error(noIpInfoErrorMsg);
   }
 }
diff --git a/src/utils/constants.js b/src/utils/constants.js
new file mode 100644
index 0000000..db1162f
--- /dev/null
+++ b/src/utils/constants.js
@@ -0,0 +1,3 @@
+const noIpInfoErrorMsg = 'no ip information';
+
+export default noIpInfoErrorMsg;
diff --git a/tests/amazonGeotarget.test.js b/tests/amazonGeotarget.test.js
index df868a2..3820f67 100644
--- a/tests/amazonGeotarget.test.js
+++ b/tests/amazonGeotarget.test.js
@@ -5,6 +5,7 @@ import amazon from 'geo-amazon';
 import fetchMock from 'fetch-mock';
 import GeolocateService from '../src/geolocate';
 import AmazonGeotargetService from '../src/amazonGeotarget';
+import noIpInfoErrorMsg from '../src/utils/constants';
 
 describe('fn whereabout', () => {
   describe('mock IP services', () => {
@@ -31,6 +32,11 @@ describe('fn whereabout', () => {
       expect(response).to.equal('US');
     }
 
+    function errorHandler(err) {
+      expect(err).to.be.an.instanceof(Error);
+      expect(err.message).to.equal('Service is not available');
+    }
+
     it('whereabout should call IPAPI when no provider specified', (done) => {
       AmazonGeotargetService.whereabout()
         .then((response) => {
@@ -72,24 +78,18 @@ describe('fn whereabout', () => {
         });
     });
 
-    it('whereabout should rejects as promised when provider 2 specified', (done) => {
+    it('whereabout should reject as promised when provider 2 specified', (done) => {
       AmazonGeotargetService.whereabout({ provider: 2 })
         .then(() => {
           assert.fail(0, 1, 'Expected rejected promise');
           done();
         }, (err) => {
-          expect(err).to.be.an.instanceof(Error);
-          expect(err.message).to.equal('Service is not available')
+          errorHandler(err);
           done();
         });
     });
 
-    it('await - whereabout should rejects as promised when provider 2 specified', async () => {
-      const errorHandler = function errorHandler(err) {
-        expect(err).to.be.an.instanceof(Error);
-        expect(err.message).to.equal('Service is not available');
-      };
-
+    it('await - whereabout should reject as promised when provider 2 specified', async () => {
       const response = await AmazonGeotargetService.whereabout({ provider: 2 })
         .catch(err => errorHandler(err));
       expect(response).to.be.an('undefined');
@@ -100,6 +100,7 @@ describe('fn whereabout', () => {
     const ipapiRes = 'US';
     const ipapiUndefined = 'Undefined';
     const freeGeoIpRes = { country_code: 'US' };
+    const freeGeoIpUndefined = '404 page not found';
 
     it('whereabout should returns US using provider 0 and IP from US is specified', async () => {
       fetchMock.get('https://ipapi.co/76.72.167.90/country', ipapiRes);
@@ -124,9 +125,9 @@ describe('fn whereabout', () => {
       }).catch((err) => {
         throw err;
       });
-      assert.strictEqual(response, undefined);
-      assert.notExists(response); // null or undefined
-      assert.isUndefined(response);
+      expect(response).to.be.an('error');
+      expect(response.message).to.be.a('string');
+      expect(response.message).to.equal(noIpInfoErrorMsg);
       fetchMock.restore();
     });
 
@@ -146,16 +147,16 @@ describe('fn whereabout', () => {
 
     it('whereabout should returns US using provider 1 and IP is non-existent', async () => {
       // freegeoip response when IP is not found
-      fetchMock.get('https://freegeoip.net/json/1234567', ' 404 page not found');
+      fetchMock.get('https://freegeoip.net/json/1234567', freeGeoIpUndefined);
       const response = await AmazonGeotargetService.whereabout({
         provider: 1,
         ip: '1234567',
       }).catch((err) => {
         throw err;
       });
-      assert.strictEqual(response, undefined);
-      assert.notExists(response); // null or undefined
-      assert.isUndefined(response);
+      expect(response).to.be.an('error');
+      expect(response.message).to.be.a('string');
+      expect(response.message).to.equal(noIpInfoErrorMsg);
       fetchMock.restore();
     });
   });
@@ -236,7 +237,7 @@ describe('fn amazonGeotarget', () => {
     });
   });
 
-  describe('stub amazonStore and geolocate ', () => {
+  describe('stub whereabout and geolocate ', () => {
     let amazonStoreStub;
     let whereaboutStub;