From 3a61e15abfc77db3061bd913135069b57015c087 Mon Sep 17 00:00:00 2001
From: jomo <github@jomo.tv>
Date: Mon, 31 Aug 2015 00:10:35 +0200
Subject: [PATCH] various networking.js improvements

- cleaned up some messy if/else code, replaced with nicely readable switch/case
- catch JSON.parse errors
---
 lib/networking.js | 59 ++++++++++++++++++++++++++++++++---------------
 test/test.js      |  2 +-
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/lib/networking.js b/lib/networking.js
index 8c3f662..0d728ae 100644
--- a/lib/networking.js
+++ b/lib/networking.js
@@ -69,7 +69,7 @@ exp.get_from_options = function(rid, url, options, callback) {
     },
     timeout: config.server.http_timeout,
     followRedirect: false,
-    encoding: (options.encoding || null),
+    encoding: options.encoding || null,
   }, function(error, response, body) {
     // log url + code + description
     var code = response && response.statusCode;
@@ -80,24 +80,34 @@ exp.get_from_options = function(rid, url, options, callback) {
       logfunc(rid, url, code, http_code[code]);
     }
 
-    // 200 or 301 depending on content type
-    if (!error && (code === 200 || code === 301)) {
-      // response received successfully
-      callback(body, response, null);
-    } else if (error) {
-      callback(body || null, response, error);
-    } else if (code === 404 || code === 204) {
-      // page does not exist
-      callback(null, response, null);
-    } else if (code === 429) {
-      // Too Many Requests exception - code 429
-      // cause error so the image will not be cached
-      callback(body || null, response, (error || "TooManyRequests"));
-    } else {
-      // Probably 500 or the likes
-      logging.error(rid, "Unexpected response:", code, body);
-      callback(body || null, response, error);
+
+    switch (code) {
+      case 200:
+      case 301:
+      case 302: // never seen, but mojang might use it in future
+      case 307: // never seen, but mojang might use it in future
+      case 308: // never seen, but mojang might use it in future
+        // these are okay
+        break;
+      case 404:
+      case 204:
+        // we don't want to cache this
+        body = null;
+        break;
+      case 429:
+        // this shouldn't usually happen, but occasionally does
+        // forcing error so it's not cached
+        error = error || "TooManyRequestsException";
+        break;
+      default:
+        if (!error) {
+          // Probably 500 or the likes
+          logging.error(rid, "Unexpected response:", code, body);
+        }
+        break;
     }
+
+    callback(body, response, error);
   });
 };
 
@@ -144,7 +154,18 @@ exp.get_profile = function(rid, uuid, callback) {
     callback(null, null);
   } else {
     exp.get_from_options(rid, session_url + uuid, { encoding: "utf8" }, function(body, response, err) {
-      callback(err || null, (body !== null ? JSON.parse(body) : null));
+      try {
+        body = body ? JSON.parse(body) : null;
+        callback(err || null, body);
+      } catch(e) {
+        if (e instanceof SyntaxError) {
+          logging.warn(rid, "Failed to parse JSON", e);
+          logging.debug(rid, body);
+          callback(err || null, null);
+        } else {
+          throw e;
+        }
+      }
     });
   }
 };
diff --git a/test/test.js b/test/test.js
index ad23e94..54b4160 100644
--- a/test/test.js
+++ b/test/test.js
@@ -1013,7 +1013,7 @@ describe("Crafatar", function() {
           it("uuid should be rate limited", function(done) {
             networking.get_profile(rid, id, function() {
               networking.get_profile(rid, id, function(err, profile) {
-                assert.strictEqual(err, "TooManyRequests");
+                assert.strictEqual(err, "TooManyRequestsException");
                 assert.strictEqual(profile.error, "TooManyRequestsException");
                 done();
               });