Fix error handling for metrics

src/server_manager/web_app/main.ts

@@ -25,14 +25,25 @@ import {ManualServerRepository} from './manual_server';
 
 const DEFAULT_SENTRY_DSN = 'https://533e56d1b2d64314bd6092a574e6d0f1@sentry.io/215496';
 
+function ensureString(queryParam: string | string[]): string | null {
+  if (!queryParam) {
+    return null;
+  }
+  if (typeof(queryParam) === 'string') {
+    return queryParam;
+  } else {
+    return queryParam[-1];
+  }
+}
+
 document.addEventListener('WebComponentsReady', () => {
   // Parse URL query params.
   const queryParams = url.parse(document.URL, true).query;
-  const debugMode = queryParams.outlineDebugMode === 'true';
-  const metricsUrl = queryParams.metricsUrl;
-  const shadowboxImage = queryParams.image;
-  const version = queryParams.version;
-  const sentryDsn = queryParams.sentryDsn || DEFAULT_SENTRY_DSN;
+  const debugMode = ensureString(queryParams.outlineDebugMode) === 'true';
+  const metricsUrl = ensureString(queryParams.metricsUrl);
+  const shadowboxImage = ensureString(queryParams.image);
+  const version = ensureString(queryParams.version);
+  const sentryDsn = ensureString(queryParams.sentryDsn) || DEFAULT_SENTRY_DSN;
 
   // Initialize error reporting.
   SentryErrorReporter.init(sentryDsn, version);

src/shadowbox/package.json

@@ -18,7 +18,7 @@
     "uuid": "^3.1.0"
   },
   "devDependencies": {
-    "@types/node": "^7.0.16",
+    "@types/node": "^8",
     "@types/randomstring": "^1.1.6",
     "@types/restify": "^2.0.41"
   }

src/shadowbox/server/ip_util.ts

@@ -33,35 +33,55 @@ export function anonymizeIp(ip: string): string {
   }
 }
 
-// Cache country lookups per IP address.
-const countryCache = new Map<string, Promise<string>>();
+export interface IpLocationService { countryForIp(ipAddress: string): Promise<string>; }
 
-export function lookupCountry(ipAddress: string) : Promise<string> {
-  if (countryCache.has(ipAddress)) {
-    // Return cached promise to prevent duplicate lookups.
-    return countryCache.get(ipAddress);
+export class FreegeoIpLocationService implements IpLocationService {
+  countryForIp(ipAddress: string): Promise<string> {
+    const countryPromise = new Promise<string>((fulfill, reject) => {
+      const options = {host: 'freegeoip.io', path: '/json/' + ipAddress};
+      https
+          .get(
+              options,
+              (response) => {
+                let body = '';
+                response.on('data', (data) => {
+                  body += data;
+                });
+                response.on('end', () => {
+                  try {
+                    const jsonResponse = JSON.parse(body);
+                    if (jsonResponse.country_code) {
+                      fulfill(jsonResponse.country_code);
+                    } else {
+                      // ZZ is user-assigned and used by CLDR for "Uknown" regions.
+                      fulfill('ZZ');
+                    }
+                  } catch (e) {
+                    reject(new Error(`Error loading country from reponse: ${e}`));
+                  }
+                });
+              })
+          .on('error', (e) => {
+            reject(new Error(`Failed to contact location service: ${e}`));
+          });
+    });
+    return countryPromise;
   }
+}
 
-  const promise = new Promise<string>((fulfill, reject) => {
-    const options = {host: 'freegeoip.io', path: '/json/' + ipAddress};
-    https.get(options, (response) => {
-      let body = '';
-      response.on('data', (data) => {
-        body += data;
-      });
-      response.on('end', () => {
-        try {
-          fulfill(JSON.parse(body).country_code);
-        } catch (err) {
-          console.error('Error loading country: ', err);
-          reject(err);
-        }
-      });
-    });
-  });
+export class CachedIpLocationService implements IpLocationService {
+  private countryCache: Map<string, Promise<string>>;
 
-  // Prevent multiple lookups of the same country.
-  countryCache.set(ipAddress, promise);
+  constructor(private locationService: IpLocationService) {
+    this.countryCache = new Map<string, Promise<string>>();
+  }
 
-  return promise;
+  countryForIp(ipAddress: string): Promise<string> {
+    if (this.countryCache.has(ipAddress)) {
+      return this.countryCache.get(ipAddress);
+    }
+    const promise = this.locationService.countryForIp(ipAddress);
+    this.countryCache.set(ipAddress, promise);
+    return promise;
+  }
 }

src/shadowbox/server/main.ts

@@ -17,12 +17,14 @@ import * as path from 'path';
 import * as process from 'process';
 import * as restify from 'restify';
 
-import * as server_config from './server_config';
-import { createManagedAccessKeyRepository } from './managed_user';
-import { ShadowsocksManagerService } from './manager_service';
+import {FilesystemTextFile} from '../infrastructure/filesystem_text_file';
+
+import * as ip_util from './ip_util';
+import {LibevShadowsocksServer} from './libev_shadowsocks_server';
+import {createManagedAccessKeyRepository} from './managed_user';
+import {ShadowsocksManagerService} from './manager_service';
 import * as metrics from './metrics';
-import { LibevShadowsocksServer } from './libev_shadowsocks_server';
-import { FilesystemTextFile } from '../infrastructure/filesystem_text_file';
+import * as server_config from './server_config';
 
 const DEFAULT_STATE_DIR = '/root/shadowbox/persisted-state';
 
@@ -50,14 +52,19 @@ function main() {
 
   const statsFilename = getPersistentFilename('shadowbox_stats.json');
   const stats = new metrics.PersistentStats(statsFilename);
+  const ipLocationService =
+      new ip_util.CachedIpLocationService(new ip_util.FreegeoIpLocationService());
   stats.onLastHourMetricsReady((startDatetime, endDatetime, lastHourUserStats) => {
     if (serverConfig.getMetricsEnabled()) {
-      metrics.getHourlyServerMetricsReport(serverConfig.serverId, startDatetime, endDatetime, lastHourUserStats)
-      .then((report) => {
-        if (report) {
-          metrics.postHourlyServerMetricsReports(report, process.env.SB_METRICS_URL);
-        }
-      });
+      metrics
+          .getHourlyServerMetricsReport(
+              serverConfig.serverId, startDatetime, endDatetime, lastHourUserStats,
+              ipLocationService)
+          .then((report) => {
+            if (report) {
+              metrics.postHourlyServerMetricsReports(report, process.env.SB_METRICS_URL);
+            }
+          });
     }
   });