From fb0c70d6488fbf783c70bbb807ab1b3b91710fd9 Mon Sep 17 00:00:00 2001
From: jomo <github@jomo.tv>
Date: Wed, 21 Oct 2015 01:02:57 +0200
Subject: [PATCH] return HTTPERROR on 429 or 5xx, fixes #151

otherwise 429 or 5xx would be overwriting cached value with null for $config minutes
---
 lib/cache.js      |  2 +-
 lib/networking.js | 12 +++++++++++-
 lib/response.js   |  2 +-
 test/test.js      |  2 +-
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/lib/cache.js b/lib/cache.js
index 61310c1..b089589 100644
--- a/lib/cache.js
+++ b/lib/cache.js
@@ -75,7 +75,7 @@ exp.info = function(callback) {
 // these 60 seconds match the duration of Mojang's rate limit ban
 // callback: error
 exp.update_timestamp = function(rid, userId, temp, callback) {
-  logging.debug(rid, "updating cache timestamp");
+  logging.debug(rid, "updating cache timestamp (" + temp + ")");
   var sub = temp ? config.caching.local - 60 : 0;
   var time = Date.now() - sub;
   // store userId in lower case if not null
diff --git a/lib/networking.js b/lib/networking.js
index 0cf7c60..704f590 100644
--- a/lib/networking.js
+++ b/lib/networking.js
@@ -77,6 +77,11 @@ exp.get_from_options = function(rid, url, options, callback) {
     var logfunc = code && code < 405 ? logging.debug : logging.warn;
     logfunc(rid, url, code || error && error.code, http_code[code]);
 
+    // not necessarily used
+    var e = new Error(code);
+    e.name = "HTTP";
+    e.code = "HTTPERROR";
+
     switch (code) {
       case 200:
       case 301:
@@ -85,13 +90,17 @@ exp.get_from_options = function(rid, url, options, callback) {
       case 308: // never seen, but mojang might use it in future
         // these are okay
         break;
+      case 204: // no content, used like 404 by mojang. making sure it really has no content
       case 404:
-      case 204:
+        // can be cached as null
+        body = null;
+        break;
       case 429: // this shouldn't usually happen, but occasionally does
       case 500:
       case 503:
       case 504:
         // we don't want to cache this
+        error = error || e;
         body = null;
         break;
       default:
@@ -99,6 +108,7 @@ exp.get_from_options = function(rid, url, options, callback) {
           // Probably 500 or the likes
           logging.error(rid, "Unexpected response:", code, body);
         }
+        error = error || e;
         body = null;
         break;
     }
diff --git a/lib/response.js b/lib/response.js
index e78947b..561d58f 100644
--- a/lib/response.js
+++ b/lib/response.js
@@ -13,7 +13,7 @@ var human_status = {
 
 
 // print these, but without stacktrace
-var silent_errors = ["ETIMEDOUT", "ESOCKETTIMEDOUT", "ECONNRESET", "EHOSTUNREACH", "ECONNREFUSED"];
+var silent_errors = ["ETIMEDOUT", "ESOCKETTIMEDOUT", "ECONNRESET", "EHOSTUNREACH", "ECONNREFUSED", "HTTPERROR"];
 
 // handles HTTP responses
 // +request+ a http.IncomingMessage
diff --git a/test/test.js b/test/test.js
index 7272664..3b04a8d 100644
--- a/test/test.js
+++ b/test/test.js
@@ -1020,7 +1020,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.ifError(err);
+                assert.strictEqual(err.toString(), "HTTP: 429");
                 assert.strictEqual(profile, null);
                 done();
               });