heroku / faceplate

A Node.js wrapper for Facebook authentication and API
76 stars 34 forks source link

Non-json response to URLs incorrectly handled #26

Open eric-wieser opened 11 years ago

eric-wieser commented 11 years ago

Got this traceback just now:

access_token=AAADrWXnFBRwBAOq2HxLgnNlD2ZC... [truncated from 56 chars for security]
^
SyntaxError: Unexpected token a
    at Object.parse (native)
    at EventEmitter.<anonymous> (/home/eric/nodejs/node_modules/faceplate/index.js)

From this line

For whatever reason, a graph URL is returning a non-JSON response fo a request to /me. I'm not sure if this is documented behaviour

eric-wieser commented 11 years ago

This is most likely caused by https://github.com/danwrong/restler/issues/110

otogen commented 11 years ago

Hello, I am a node.js & express.js beginner I got a similar error below today. The program is going to clash after that. It is very weired because I have never got this error before. I am using Facebook-Template-Nodejs including Faceplate 0.0.4 , Restler 2.0.0, and b64url 1.0.3 I have tried npm install restler2.0.1, but could not get solution.

Please tell me the solution. Is it cause from graph API and JSON data?

Thank you for your thoughts !

undefined:1 SyntaxError:Failed to parse JSON body:Unexpected token o

SyntaxError: Unexpected token S at Object.parse(native) at EventEmitter.FaceplateSession.get(../node_modules/faceplate/index.js:121:25) at EventEmitter.emit(events.js:99:17) . . .

eric-wieser commented 11 years ago

@otogen: Did you read the comment I made 3 days ago? This is due to an unfixed bug in restler.

otogen commented 11 years ago

Totally yes

Henddher commented 11 years ago

I "was" experiencing this issue with Heroku and facebook. I updated to faceplate 0.5.0, plus the latest changes from Eric Weiser and yet faceplate's req.facebook.app(...) failed as well as fql and get. It is caused by JSON.parse "hanging" (never returns but an exception is thrown). I "resolved" the issue by calling JSON.stringify on the payload from restler before calling JSON.parse. It might be redundant but the issue appears to be caused by Facebook sending JSON payload with field names containing underscore characters?

Here is a sample of the change I made: (// <<<<<) Does that make sense at all?

this.get = function(path, params, cb) {
if (cb === undefined) {
cb = params;
params = {};
}
if (self.token)
params.access_token = self.token;
try {
var request = restler.get('https://graph.facebook.com' + path,
{ query: params });
request.on('fail', function(data) {
var result = JSON.parse(JSON.stringify(data)); // <<<<<<<<<<
cb(result);
});
request.on('success', function(data) {
var result = JSON.parse(JSON.stringify(data)); // <<<<<<<<<<
cb(null, result);
});
} catch (err) {
cb(err);
}
};

Henddher commented 11 years ago

And with these 2 changes, login seems to work also: (Are these changes correct?)

// not logged in or not authorized
if (!data.user_id) {
  cb(data); // <<<<<<<<
  return;
}

if (data.access_token || data.oauth_token) {
  cb(data); // <<<<<<<<<
  return;
}

It seems to me that the parse_signed_request function is not in synch (API-wise) with FaceplateSession "constructor" and the chain of callbacks from middleware. Maybe?

otogen commented 11 years ago

Thank you Henddher San ! I will check it soon and report you here soon !

otogen commented 11 years ago

Henddher San, I have just check the program and it works ! In my case, if faceplate would be upgraded to 0.5.0, then Facebook-template-nodejs does not work. But without change, your comments did work.

So great, thank you so much Henddher San !!!

Henddher commented 11 years ago

Machunter (in another post https://github.com/danwrong/restler/issues/114) confirmed the issue we are seeing: JSON.parse failing to parse the response. Thus, JSON.stringify followed by JSON.parse "solves" the problem.

Another strange thing is that adding JSON.stringify to the parse_signed_request function does NOT work. JSON.stringify seems to be "needed" for function get and fql only.

Can anyone knowledgable with Restler and Faceplate share some thoughts about the "solution" we are discussing? ("solution" between quotes because I do NOT know if it's correct - honestly, it makes no sense as solution)

I am no expert of neither Restler, Faceplate, nor Node as a matter of fact. The most strange thing, IMO, it's the fact that the issue started happening without changes to the code of the apps. So, either Facebook responses changed or Node & NPM versions used on Heroku changed (to a different version which exposes faceplate and restler issues? - Node 0.10?).

In my package.json, I had declared engines as follows:

"engines": { "node": "0.8.x", "npm": "1.1.x" }

That declaration apparently allows newer versions of Node and NPM to be used if available. Now that I know which exact versions are being used, I modified my package.json as follows:

"engines": { "node": "0.8.21", "npm": "1.1.65" }

However, I don't know if Heroku infrastructure honors specific versions either.

Can anyone share some light about JSON.parse?

I still think there's an issue with faceplate MASTER/HEAD and how "Errors" are created and passed onto FaceplateSession constructor when these errors arise during Restler and/or JSON.parse operations.

eric-wieser commented 11 years ago

I don't know which version of node made it actually break, but danwrong/restler#110 has been wrong forever - Every instance of a restler handler you create shares the same EventEmitter. Why I got my error:

  1. create two requests (i.e., an auth request and an API request)
  2. Add an event handler to each
  3. When either completes both handlers fire - my API request handler recieves a non-json auth-token, and everything fails.

I'm not sure what you're seeing here, but this bug is fixed for me after applying danwrong/restler#113 and using faceplate/master.

eric-wieser commented 11 years ago

@Henddher:

(Are these changes correct?)

// not logged in or not authorized
if (!data.user_id) {
  cb(data); // <<<<<<<<
  return;
}

if (data.access_token || data.oauth_token) {
  cb(data); // <<<<<<<<<
  return;
}

Nope. All you're doing there is reverting the changes made to faceplate/0.5.0. These changes are sound - the problem is the other code, which I fixed in #22, #23, and #25. You'll need to update your API calls - 0.5.0 is an API-incompatible update.

Henddher commented 11 years ago

Thank you Eric.

How can I use Faceplate master and danwrong/restler#113

I am using Heroku but I do NOT know if Heroku honors node module versions specified in package.json

Anyone knows?

How could I specify these 2 different versions of Faceplate and Restler?

"dependencies": { "express": "3.1.0", "jade": "0.28.1", "stylus": "0.32.0", "faceplate": "0.5.0", "async": "0.1.18", "ejs": "0.4.3", "restler": "2.0.0", "b64url": "1.0.3", "pg" : "0.6.15" },

"engines": { "node": "0.8.21", "npm": "1.1.65" }

eric-wieser commented 11 years ago

@Henddher: I can't comment - I haven't been testing with node 0.8.21.

Most likely, your problem is that 0.5.0 is a non-working release of faceplate, and the commits I made on top haven't been put into a 0.5.1 on npm. Until then, you could revert to 0.4.

Henddher commented 11 years ago

Thanks Eric. The strange thing is that my code stopped working when I was still using faceplate 0.0.4. Out of the blue, three apps started failing. Even older apps I had not touched in weeks are also failing now.

My package.json had 0.8.x and 1.1.x initially. I even tried node 0.8.21 and 0.8.19 and the app still fails with the same JSON.parse issue.

Either Heroku does not honor package.json versions of dependencies and/or engines or Facebook JSON responses changed somehow.

I can't think of any other possible explanation for the sudden breakage.

Only calling JSON.stringify before JSON.parse seems to successfully solve the issue.

Keep in mind that I tested creating a brand new Heroku app from Facebook developer site and this sample app ALSO fails exactly the same way as the others.

Are there any combination of versions (node, faceplate and restler) in which the issue does not happen?

louiseknight commented 11 years ago

I've been having the problem since last week and yet I don't have any calls to JSON.parse() in my code. I'm really no expert with Heroku or Node.js but I tried looking for code similar to the excerpt given in the solution posted, and I could find references to get and fql; I changed my code like so:

function handle_facebook_request(req, res) {

  // if the user is logged in
  if (req.facebook.token) {

 async.parallel([
  function(cb) {
    // query 4 friends and send them to the socket for this socket id
    req.facebook.get('/me/friends', { limit: 4 }, function(friends) {
      req.friends = JSON.parse(JSON.stringify(friends)); // was previously req.friends = friends;
      cb();
    });
  },
  function(cb) {
    // query 16 photos and send them to the socket for this socket id
    req.facebook.get('/me/photos', { limit: 16 }, function(photos) {
      req.photos = JSON.parse(JSON.stringify(photos)); // was previously req.photos = photos;
      cb();
    });
  },
  function(cb) {
    // query 4 likes and send them to the socket for this socket id
    req.facebook.get('/me/likes', { limit: 4 }, function(likes) {
       req.likes = JSON.parse(JSON.stringify(likes)); // was previously req.likes = likes;
      cb();
    });
  },
  function(cb) {
    // use fql to get a list of my friends that are using this app
     req.facebook.fql('SELECT uid, name, is_app_user, pic_square FROM user WHERE uid in (SELECT uid2 FROM friend WHERE uid1 = me()) AND is_app_user = 1', function(result) {
      req.friends_using_app = JSON.parse(JSON.stringify(result)); // was previously req.friends_using_app = result;
      cb();
    });
  }
], function() {
  render_page(req, res);
});

  } else {
    render_page(req, res);
  }
}

This was the only code I could find with get and fql calls, apart from the below, which I haven't changed:

app.get('/', handle_facebook_request);
app.post('/', handle_facebook_request);

The changes I did make haven't worked, but there were no calls to JSON.parse() in my code before, so I wasn't sure what to do; I do have calls to JSON.stringify() for working with my Postgres database, but I've never had a problem with those before... (but could they have caused a problem?)

Henddher commented 11 years ago

Actually, the changes I produced to bypass the problem are not done to the "example heroku app" but to Faceplate itself. IOW, you need to copy faceplate module to your "example app" and add the JSON.stringify to the JSON.parse calls - not all of them but only the ones I cited. To use my own version of faceplate, I simply copied faceplate.js (from faceplate node module) onto my app source tree and updated the "require('faceplate')" statement in the example app to point to my own copy of faceplate. I am varely familiar with Node.js myself so I don't know if there are better ways to do this.

Keep in mind that this is NOT the real solution. According to Eric Wieser, the real issue lies in Restler.

Why this issue happens? I truly don't know either. I speculate this started happening because something changed in Facebook side and its graph responses are different causing the Restler issue to appear.

louiseknight commented 11 years ago

Ok, so here is what I've done:

and index.js under node_modules/faceplate (http://pastebin.com/dsbpuYbC).

Henddher commented 11 years ago

This is the faceplate.js I am using. I placed it in the same dir where web.js is at. This is a frankenstein version of faceplate and I suggest you take the one from 0.0.4 instead. This one below is 0.5.0 plus some other changes and yet NOT stable. Apparently, 0.0.4 is stable. The interesting changes are marked with // <<<<

var b64url  = require('b64url');
var crypto  = require('crypto');
var qs      = require('querystring');
var restler = require('restler');

var Faceplate = function(options) {

  var self = this;

  this.options = options || {};
  this.app_id  = this.options.app_id;
  this.secret  = this.options.secret;

  this.middleware = function() {
    return function(req, res, next) {
      if (req.body.signed_request) {
        self.parse_signed_request(req.body.signed_request, function(decoded_signed_request) {
          req.facebook = new FaceplateSession(self, decoded_signed_request);
          next();
        });
      } else if (req.cookies["fbsr_" + self.app_id]) {
        self.parse_signed_request(req.cookies["fbsr_" + self.app_id], function(decoded_signed_request) {
          req.facebook = new FaceplateSession(self, decoded_signed_request);
          next();
        });
      } else {
        req.facebook = new FaceplateSession(self);
        next();
      }
    };
  };

  this.parse_signed_request = function(signed_request, cb) {
    var encoded_data = signed_request.split('.', 2);

    var sig  = encoded_data[0];
    var json = b64url.decode(encoded_data[1]);
    var data = JSON.parse(json);

    // check algorithm
    if (!data.algorithm || (data.algorithm.toUpperCase() != 'HMAC-SHA256')) {
      cb(new Error("unknown algorithm. expected HMAC-SHA256"));
      return;
    }

    // check signature
    var secret = self.secret;
    var expected_sig = crypto.createHmac('sha256', secret).update(encoded_data[1]).digest('base64').replace(/\+/g,'-').replace(/\//g,'_').replace('=','');

    if (sig !== expected_sig) {
      cb(new Error("bad signature"));
      return;
    }

    // not logged in or not authorized
    if (!data.user_id) {
      cb(data);
      return;
    }

    if (data.access_token || data.oauth_token) {
      cb(data);
      return;
    }

    if (!data.code) {
      cb(new Error("no oauth token and no code to get one"));
      return;
    }

    var params = {
      client_id:     self.app_id,
      client_secret: self.secret,
      redirect_uri:  '',
      code:          data.code
    };

    var request = restler.get('https://graph.facebook.com/oauth/access_token',
      { query:params });

    request.on('fail', function(data) {
      var result = JSON.parse(JSON.stringify(data)); // <<<<
      cb(result);
    });

    request.on('success', function(data) {
      cb(qs.parse(data));
    });
  };
};

var FaceplateSession = function(plate, signed_request) {

  var self = this;

  this.plate = plate;
  if (signed_request) {
      this.token  = signed_request.access_token || signed_request.oauth_token;
      this.signed_request = signed_request;
  }

  this.app = function(cb) {
    self.get('/' + self.plate.app_id, function(err, app) {
      cb(err,app);
    });
  };

  this.me = function(cb) {
    if (self.token) {
      self.get('/me', function(err, me) {
        cb(err,me);
      });
    } else {
      cb(null,null);
    }
  };

  this.get = function(path, params, cb) {
    if (cb === undefined) {
      cb = params;
      params = {};
    }

    if (self.token)
      params.access_token = self.token;

    try {
        var request = restler.get('https://graph.facebook.com' + path,
          { query: params });
        request.on('fail', function(data) {
          var result = JSON.parse(JSON.stringify(data)); // <<<<
          cb(result);
        });
        request.on('success', function(data) {
          var result = JSON.parse(JSON.stringify(data)); // <<<<
          cb(null, result);
        });
    } catch (err) {
      cb(err);
    }
  };

  this.fql = function(query, cb) {
    var params = { access_token: self.token, format:'json' };
    var method;
    var onComplete;

    if (typeof query == 'string') {
      method = 'fql.query';
      params.query = query;
      onComplete = function(res){
        var result = JSON.parse(JSON.stringify(res)); // <<<<
        cb(null, result.data ? result.data : result);
      };
    }
    else {
      method = 'fql.multiquery';
      params.queries = JSON.stringify(query);
      onComplete = function(res) {
        if (res.error_code)
          return cb(res);

        var data = {};
        res.forEach(function(q) {
          data[q.name] = q.fql_result_set;
        });
        cb(null,data);
      };
    }
    var request = restler.get('https://api.facebook.com/method/'+method,
      { query: params });
    request.on('fail', function(data) {
      var result = JSON.parse(JSON.stringify(data)); // <<<<
      cb(result);
    });
    request.on('success', onComplete);
  };

  this.post = function (params, cb) {
    var request = restler.post(
      'https://graph.facebook.com/me/feed',
      {query: {access_token: self.token}, data: params}
      );
    request.on('fail', function(data) {
      var result = JSON.parse(JSON.stringify(data)); // <<<<
      cb(result);
    });
    request.on('success', function (data) {
      var result = JSON.parse(JSON.stringify(data)); // <<<<
      cb(null, result.data ? result.data : result);
    });
  };
};

module.exports.middleware = function(options) {
  return new Faceplate(options).middleware();
};

Then, in web.js:

var faceplate = require('./faceplate');

This declaration will make the app to use the hacked faceplate.js instead of the real one in node_modules which comes from package.json dependency.

IMPORTANT: This is what works for me. I cannot guarantee that it will work for anyone else.

louiseknight commented 11 years ago

Thank you! It finally works! :D

I used the 0.0.4 version (which is quite different in some parts), so if anyone wants that version with changes made here you go (http://pastebin.com/FfV3eJSx). Also, it should be noted that it works if you make the changes to the index.js in node_modules/faceplate rather than making a copy (which didn't work for me).

Thanks again - this issue has been bugging me for over a week!