Skip to content

catch JSON parsing errors in fbgraph #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eawooten wants to merge 5 commits into
base: master
Choose a base branch
from
Open

catch JSON parsing errors in fbgraph #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
81f4aef
catch JSON parsing errors in fbgraph
eawooten Mar 19, 2026
afe133e
Update lib/graph.js
eawooten Mar 19, 2026
c70605e
Update lib/graph.js
eawooten Mar 19, 2026
202252d
Update tests/graph.test.js
eawooten Mar 19, 2026
1b12815
Update tests/graph.test.js
eawooten Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions lib/graph.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,17 @@ Graph.prototype.get = function () {
return;
}

body = { ...JSON.parse(body), headers: res.headers };
if (~res.headers['content-type'].indexOf('image')) {
if (res.headers['content-type'] && ~res.headers['content-type'].indexOf('image')) {
body = {
image: true
, headers: res.headers
image: true
, headers: res.headers
};
} else {
try {
body = { ...JSON.parse(body), headers: res.headers };
} catch (e) {
// parsing failed, pass raw body to end()
}
}

self.end(body);
Expand Down Expand Up @@ -233,7 +238,11 @@ Graph.prototype.post = function() {
return;
}

body = { ...JSON.parse(body), headers: res.headers };
try {
body = { ...JSON.parse(body), headers: res.headers };
} catch (e) {
// if parsing fails, we pass the raw body to end()
}
self.end(body);
})
.on('error', (err) => {
Expand Down
28 changes: 27 additions & 1 deletion tests/graph.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ var graph = require("../index")
, FBConfig = require("./config").facebook
, vows = require("vows")
, events = require("events")
, assert = require("assert");
, assert = require("assert")
, request = require("request");


var testUser1 = {}
Expand Down Expand Up @@ -277,6 +278,31 @@ vows.describe("graph.test").addBatch({
}
}
}
}).addBatch({
"Hardening JSON Parsing": {
"When receiving a non-JSON response": {
topic: function () {
var callback = this.callback
, originalGet = request.get;

request.get = function (options, cb) {
Copy link
Copy Markdown

@pmg-sevenonethree pmg-sevenonethree Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to somehow reset this back to what it was?

Copy link
Copy Markdown
Author

@eawooten eawooten Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that's line 296 below

var res = { headers: { 'content-type': 'text/html' } }
, body = '{"foo": bar}'; // Malformed JSON
setImmediate(function () { cb(null, res, body); });
return { on: function () { return this; } };
};

graph.get('/me', function (err, res) {
request.get = originalGet;
callback(err, res);
});
},
"it should return an error instead of crashing": function (err, res) {
assert.isNotNull(err);
assert.equal(err.message, 'Error parsing json');
}
}
}
}).addBatch({
"When tests are over": {
topic: function () {
Expand Down