call back after writing to cache, fixes #86

remove_hash has no callback because it's only called very rarely and the write/change calls can cause more trouble

modules/cache.js

@@ -93,17 +93,21 @@ exp.info = function(callback) {
 };
 
 // sets the timestamp for +userId+ and its face file's date to now
-exp.update_timestamp = function(rid, userId, hash) {
+// +callback+ contains error
+exp.update_timestamp = function(rid, userId, hash, callback) {
   logging.log(rid + "cache: updating timestamp");
   var time = new Date().getTime();
   // store userId in lower case if not null
   userId = userId && userId.toLowerCase();
-  redis.hmset(userId, "t", time);
+  redis.hmset(userId, "t", time, function(err) {
+    callback(err);
+  });
   update_file_date(rid, hash);
 };
 
 // create the key +userId+, store +skin_hash+ hash, +cape_hash+ hash and time
-exp.save_hash = function(rid, userId, skin_hash, cape_hash) {
+// +callback+ contans error
+exp.save_hash = function(rid, userId, skin_hash, cape_hash, callback) {
   logging.log(rid + "cache: saving hash");
   logging.log(rid + "skin:" + skin_hash + " cape:" + cape_hash);
   var time = new Date().getTime();
@@ -112,9 +116,12 @@ exp.save_hash = function(rid, userId, skin_hash, cape_hash) {
   cape_hash = cape_hash || ".";
   // store userId in lower case if not null
   userId = userId && userId.toLowerCase();
-  redis.hmset(userId, "s", skin_hash, "c", cape_hash, "t", time);
+  redis.hmset(userId, "s", skin_hash, "c", cape_hash, "t", time, function(err){
+    callback(err);
+  });
 };
 
+// removes the hash for +userId+ from the cache
 exp.remove_hash = function(rid, userId) {
   logging.log(rid + "cache: deleting hash");
   redis.del(userId.toLowerCase(), "h", "t");

modules/helpers.js

@@ -20,8 +20,9 @@ function store_skin(rid, userId, profile, details, callback) {
     if (url) {
       var skin_hash = get_hash(url);
       if (details && details.skin === skin_hash) {
-        cache.update_timestamp(rid, userId, skin_hash);
-        callback(null, skin_hash);
+        cache.update_timestamp(rid, userId, skin_hash, function(err) {
+          callback(err, skin_hash);
+        });
       } else {
         logging.log(rid + "new skin hash: " + skin_hash);
         var facepath = __dirname + "/../" + config.faces_dir + skin_hash + ".png";
@@ -64,8 +65,9 @@ function store_cape(rid, userId, profile, details, callback) {
     if (url) {
       var cape_hash = get_hash(url);
       if (details && details.cape === cape_hash) {
-        cache.update_timestamp(rid, userId, cape_hash);
-        callback(null, cape_hash);
+        cache.update_timestamp(rid, userId, cape_hash, function(err) {
+          callback(err, cape_hash);
+        });
       } else {
         logging.log(rid + "new cape hash: " + cape_hash);
         var capepath = __dirname + "/../" + config.capes_dir + cape_hash + ".png";
@@ -131,7 +133,7 @@ function deep_property_check(arr, property, value) {
 // downloads the images for +userId+ while checking the cache
 // status based on +details+. +type+ specifies which
 // image type should be called back on
-// +callback+ contains the error buffer and image hash
+// +callback+ contains error, image hash
 function store_images(rid, userId, details, type, callback) {
   var is_uuid = userId.length > 16;
   var new_hash = {
@@ -147,11 +149,13 @@ function store_images(rid, userId, details, type, callback) {
         callback_for(userId, type, err, null);
       } else {
         store_skin(rid, userId, profile, details, function(err, skin_hash) {
-          cache.save_hash(rid, userId, skin_hash, null);
-          callback_for(userId, "skin", err, skin_hash);
-          store_cape(rid, userId, profile, details, function(err, cape_hash) {
-            cache.save_hash(rid, userId, skin_hash, cape_hash);
-            callback_for(userId, "cape", err, cape_hash);
+          cache.save_hash(rid, userId, skin_hash, null, function(cache_err) {
+            callback_for(userId, "skin", (err || cache_err), skin_hash);
+            store_cape(rid, userId, profile, details, function(err, cape_hash) {
+              cache.save_hash(rid, userId, skin_hash, cape_hash, function(cache_err) {
+                callback_for(userId, "cape", (err || cache_err), cape_hash);
+              });
+            });
           });
         });
       }

test/test.js

@@ -163,9 +163,10 @@ describe("Crafatar", function() {
     });
     it("should ignore file updates on invalid files", function(done) {
       assert.doesNotThrow(function() {
-        cache.update_timestamp(rid, "0123456789abcdef0123456789abcdef", "invalid-file.png");
+        cache.update_timestamp(rid, "0123456789abcdef0123456789abcdef", "invalid-file.png", function(err) {
+          done();
+        });
       });
-      done();
     });
     it("should not find the file", function(done) {
       skins.open_skin(rid, 'non/existant/path', function(err, img) {