Browse Source

return HTTPERROR on 429 or 5xx, fixes #151

otherwise 429 or 5xx would be overwriting cached value with null for $config minutes
jomo 9 years ago
parent
commit
fb0c70d648
4 changed files with 14 additions and 4 deletions
  1. 1 1
      lib/cache.js
  2. 11 1
      lib/networking.js
  3. 1 1
      lib/response.js
  4. 1 1
      test/test.js

+ 1 - 1
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

+ 11 - 1
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;
     }

+ 1 - 1
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

+ 1 - 1
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();
               });