GitHub PullRequestReviewCommentEvent at 2014-07-22 19:03:33
Repository: node-feedparser
Actor: alltom
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/danmactough/node-feedparser/pulls/comments/15248638","id":15248638,"diff_hunk":"@@ -1020,13 +1020,31 @@ FeedParser.prototype.handleItem = function handleItem (node, type, options){\n \n // Naive Stream API\n FeedParser.prototype._transform = function (data, encoding, done) {\n-  this.stream.write(data);\n-  done();\n+  var ex = tryCatch(function () {\n+    this.stream.write(data);\n+  }, this);\n+  if (ex) {\n+    this.stream._parser.onerror.call(this.stream, ex);\n+  }\n+  done(ex);\n };\n \n FeedParser.prototype._flush = function (done) {\n-  this.stream.end();\n-  done();\n+  var ex = tryCatch(function () {\n+    this.stream.end();\n+  }, this);\n+  done(ex);\n };\n \n exports = module.exports = FeedParser;\n+\n+function tryCatch (func, ctx) {","path":"main.js","position":26,"original_position":26,"commit_id":"ce9d04e32c078239ef2583f0104e7553dcd26507","original_commit_id":"ce9d04e32c078239ef2583f0104e7553dcd26507","user":{"login":"alltom","id":1678,"avatar_url":"https://avatars.githubusercontent.com/u/1678?","gravatar_id":"68971fe7ee8fd814c26391c727c065bb","url":"https://api.github.com/users/alltom","html_url":"https://github.com/alltom","followers_url":"https://api.github.com/users/alltom/followers","following_url":"https://api.github.com/users/alltom/following{/other_user}","gists_url":"https://api.github.com/users/alltom/gists{/gist_id}","starred_url":"https://api.github.com/users/alltom/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alltom/subscriptions","organizations_url":"https://api.github.com/users/alltom/orgs","repos_url":"https://api.github.com/users/alltom/repos","events_url":"https://api.github.com/users/alltom/events{/privacy}","received_events_url":"https://api.github.com/users/alltom/received_events","type":"User","site_admin":false},"body":"Having `try{}` in a function prevents some optimizations, so sometimes you want to put the `try{}` in a separate function where optimization doesn't matter.","created_at":"2014-07-22T19:03:33Z","updated_at":"2014-07-22T19:03:33Z","html_url":"https://github.com/danmactough/node-feedparser/pull/119#discussion_r15248638","pull_request_url":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119","_links":{"self":{"href":"https://api.github.com/repos/danmactough/node-feedparser/pulls/comments/15248638"},"html":{"href":"https://github.com/danmactough/node-feedparser/pull/119#discussion_r15248638"},"pull_request":{"href":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119"}}},"pull_request":{"url":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119","id":18731884,"html_url":"https://github.com/danmactough/node-feedparser/pull/119","diff_url":"https://github.com/danmactough/node-feedparser/pull/119.diff","patch_url":"https://github.com/danmactough/node-feedparser/pull/119.patch","issue_url":"https://api.github.com/repos/danmactough/node-feedparser/issues/119","number":119,"state":"open","title":"Wrap sax in trycatch","user":{"login":"danmactough","id":357481,"avatar_url":"https://avatars.githubusercontent.com/u/357481?","gravatar_id":"333fbe590fbedcff27478befe4bbbdcd","url":"https://api.github.com/users/danmactough","html_url":"https://github.com/danmactough","followers_url":"https://api.github.com/users/danmactough/followers","following_url":"https://api.github.com/users/danmactough/following{/other_user}","gists_url":"https://api.github.com/users/danmactough/gists{/gist_id}","starred_url":"https://api.github.com/users/danmactough/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danmactough/subscriptions","organizations_url":"https://api.github.com/users/danmactough/orgs","repos_url":"https://api.github.com/users/danmactough/repos","events_url":"https://api.github.com/users/danmactough/events{/privacy}","received_events_url":"https://api.github.com/users/danmactough/received_events","type":"User","site_admin":false},"body":"Resolves #112 by wrapping calls to sax's write and end methods in a try/catch. If we catch an exception, we trigger the exception on the sax parser -- from where it would have originated had the parser been catching parse errors.\r\n\r\nThis is not complete. The error-handling is currently suboptimal, as it will trigger an error and then never end parsing without user intervention (see L#58 in the test). So, I'm considering making `this.push(null)` automatic.\r\n\r\n@kof @alltom Any thoughts?","created_at":"2014-07-22T18:25:28Z","updated_at":"2014-07-22T19:03:33Z","closed_at":null,"merged_at":null,"merge_commit_sha":"795734b5858796139e081092df914d4be4bbc66e","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119/commits","review_comments_url":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119/comments","review_comment_url":"https://api.github.com/repos/danmactough/node-feedparser/pulls/comments/{number}","comments_url":"https://api.github.com/repos/danmactough/node-feedparser/issues/119/comments","statuses_url":"https://api.github.com/repos/danmactough/node-feedparser/statuses/ce9d04e32c078239ef2583f0104e7553dcd26507","head":{"label":"danmactough:wrap-sax-in-trycatch","ref":"wrap-sax-in-trycatch","sha":"ce9d04e32c078239ef2583f0104e7553dcd26507","user":{"login":"danmactough","id":357481,"avatar_url":"https://avatars.githubusercontent.com/u/357481?","gravatar_id":"333fbe590fbedcff27478befe4bbbdcd","url":"https://api.github.com/users/danmactough","html_url":"https://github.com/danmactough","followers_url":"https://api.github.com/users/danmactough/followers","following_url":"https://api.github.com/users/danmactough/following{/other_user}","gists_url":"https://api.github.com/users/danmactough/gists{/gist_id}","starred_url":"https://api.github.com/users/danmactough/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danmactough/subscriptions","organizations_url":"https://api.github.com/users/danmactough/orgs","repos_url":"https://api.github.com/users/danmactough/repos","events_url":"https://api.github.com/users/danmactough/events{/privacy}","received_events_url":"https://api.github.com/users/danmactough/received_events","type":"User","site_admin":false},"repo":{"id":2392967,"name":"node-feedparser","full_name":"danmactough/node-feedparser","owner":{"login":"danmactough","id":357481,"avatar_url":"https://avatars.githubusercontent.com/u/357481?","gravatar_id":"333fbe590fbedcff27478befe4bbbdcd","url":"https://api.github.com/users/danmactough","html_url":"https://github.com/danmactough","followers_url":"https://api.github.com/users/danmactough/followers","following_url":"https://api.github.com/users/danmactough/following{/other_user}","gists_url":"https://api.github.com/users/danmactough/gists{/gist_id}","starred_url":"https://api.github.com/users/danmactough/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danmactough/subscriptions","organizations_url":"https://api.github.com/users/danmactough/orgs","repos_url":"https://api.github.com/users/danmactough/repos","events_url":"https://api.github.com/users/danmactough/events{/privacy}","received_events_url":"https://api.github.com/users/danmactough/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/danmactough/node-feedparser","description":"Robust RSS, Atom, and RDF feed parsing in Node.js","fork":false,"url":"https://api.github.com/repos/danmactough/node-feedparser","forks_url":"https://api.github.com/repos/danmactough/node-feedparser/forks","keys_url":"https://api.github.com/repos/danmactough/node-feedparser/keys{/key_id}","collaborators_url":"https://api.github.com/repos/danmactough/node-feedparser/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/danmactough/node-feedparser/teams","hooks_url":"https://api.github.com/repos/danmactough/node-feedparser/hooks","issue_events_url":"https://api.github.com/repos/danmactough/node-feedparser/issues/events{/number}","events_url":"https://api.github.com/repos/danmactough/node-feedparser/events","assignees_url":"https://api.github.com/repos/danmactough/node-feedparser/assignees{/user}","branches_url":"https://api.github.com/repos/danmactough/node-feedparser/branches{/branch}","tags_url":"https://api.github.com/repos/danmactough/node-feedparser/tags","blobs_url":"https://api.github.com/repos/danmactough/node-feedparser/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/danmactough/node-feedparser/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/danmactough/node-feedparser/git/refs{/sha}","trees_url":"https://api.github.com/repos/danmactough/node-feedparser/git/trees{/sha}","statuses_url":"https://api.github.com/repos/danmactough/node-feedparser/statuses/{sha}","languages_url":"https://api.github.com/repos/danmactough/node-feedparser/languages","stargazers_url":"https://api.github.com/repos/danmactough/node-feedparser/stargazers","contributors_url":"https://api.github.com/repos/danmactough/node-feedparser/contributors","subscribers_url":"https://api.github.com/repos/danmactough/node-feedparser/subscribers","subscription_url":"https://api.github.com/repos/danmactough/node-feedparser/subscription","commits_url":"https://api.github.com/repos/danmactough/node-feedparser/commits{/sha}","git_commits_url":"https://api.github.com/repos/danmactough/node-feedparser/git/commits{/sha}","comments_url":"https://api.github.com/repos/danmactough/node-feedparser/comments{/number}","issue_comment_url":"https://api.github.com/repos/danmactough/node-feedparser/issues/comments/{number}","contents_url":"https://api.github.com/repos/danmactough/node-feedparser/contents/{+path}","compare_url":"https://api.github.com/repos/danmactough/node-feedparser/compare/{base}...{head}","merges_url":"https://api.github.com/repos/danmactough/node-feedparser/merges","archive_url":"https://api.github.com/repos/danmactough/node-feedparser/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/danmactough/node-feedparser/downloads","issues_url":"https://api.github.com/repos/danmactough/node-feedparser/issues{/number}","pulls_url":"https://api.github.com/repos/danmactough/node-feedparser/pulls{/number}","milestones_url":"https://api.github.com/repos/danmactough/node-feedparser/milestones{/number}","notifications_url":"https://api.github.com/repos/danmactough/node-feedparser/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/danmactough/node-feedparser/labels{/name}","releases_url":"https://api.github.com/repos/danmactough/node-feedparser/releases{/id}","created_at":"2011-09-15T14:36:25Z","updated_at":"2014-07-22T15:05:58Z","pushed_at":"2014-07-22T18:16:39Z","git_url":"git://github.com/danmactough/node-feedparser.git","ssh_url":"git@github.com:danmactough/node-feedparser.git","clone_url":"https://github.com/danmactough/node-feedparser.git","svn_url":"https://github.com/danmactough/node-feedparser","homepage":"","size":1193,"stargazers_count":568,"watchers_count":568,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":81,"mirror_url":null,"open_issues_count":9,"forks":81,"open_issues":9,"watchers":568,"default_branch":"master"}},"base":{"label":"danmactough:master","ref":"master","sha":"7f1a82ca934f4d42ab681211c9c21792e10e2dfb","user":{"login":"danmactough","id":357481,"avatar_url":"https://avatars.githubusercontent.com/u/357481?","gravatar_id":"333fbe590fbedcff27478befe4bbbdcd","url":"https://api.github.com/users/danmactough","html_url":"https://github.com/danmactough","followers_url":"https://api.github.com/users/danmactough/followers","following_url":"https://api.github.com/users/danmactough/following{/other_user}","gists_url":"https://api.github.com/users/danmactough/gists{/gist_id}","starred_url":"https://api.github.com/users/danmactough/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danmactough/subscriptions","organizations_url":"https://api.github.com/users/danmactough/orgs","repos_url":"https://api.github.com/users/danmactough/repos","events_url":"https://api.github.com/users/danmactough/events{/privacy}","received_events_url":"https://api.github.com/users/danmactough/received_events","type":"User","site_admin":false},"repo":{"id":2392967,"name":"node-feedparser","full_name":"danmactough/node-feedparser","owner":{"login":"danmactough","id":357481,"avatar_url":"https://avatars.githubusercontent.com/u/357481?","gravatar_id":"333fbe590fbedcff27478befe4bbbdcd","url":"https://api.github.com/users/danmactough","html_url":"https://github.com/danmactough","followers_url":"https://api.github.com/users/danmactough/followers","following_url":"https://api.github.com/users/danmactough/following{/other_user}","gists_url":"https://api.github.com/users/danmactough/gists{/gist_id}","starred_url":"https://api.github.com/users/danmactough/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danmactough/subscriptions","organizations_url":"https://api.github.com/users/danmactough/orgs","repos_url":"https://api.github.com/users/danmactough/repos","events_url":"https://api.github.com/users/danmactough/events{/privacy}","received_events_url":"https://api.github.com/users/danmactough/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/danmactough/node-feedparser","description":"Robust RSS, Atom, and RDF feed parsing in Node.js","fork":false,"url":"https://api.github.com/repos/danmactough/node-feedparser","forks_url":"https://api.github.com/repos/danmactough/node-feedparser/forks","keys_url":"https://api.github.com/repos/danmactough/node-feedparser/keys{/key_id}","collaborators_url":"https://api.github.com/repos/danmactough/node-feedparser/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/danmactough/node-feedparser/teams","hooks_url":"https://api.github.com/repos/danmactough/node-feedparser/hooks","issue_events_url":"https://api.github.com/repos/danmactough/node-feedparser/issues/events{/number}","events_url":"https://api.github.com/repos/danmactough/node-feedparser/events","assignees_url":"https://api.github.com/repos/danmactough/node-feedparser/assignees{/user}","branches_url":"https://api.github.com/repos/danmactough/node-feedparser/branches{/branch}","tags_url":"https://api.github.com/repos/danmactough/node-feedparser/tags","blobs_url":"https://api.github.com/repos/danmactough/node-feedparser/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/danmactough/node-feedparser/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/danmactough/node-feedparser/git/refs{/sha}","trees_url":"https://api.github.com/repos/danmactough/node-feedparser/git/trees{/sha}","statuses_url":"https://api.github.com/repos/danmactough/node-feedparser/statuses/{sha}","languages_url":"https://api.github.com/repos/danmactough/node-feedparser/languages","stargazers_url":"https://api.github.com/repos/danmactough/node-feedparser/stargazers","contributors_url":"https://api.github.com/repos/danmactough/node-feedparser/contributors","subscribers_url":"https://api.github.com/repos/danmactough/node-feedparser/subscribers","subscription_url":"https://api.github.com/repos/danmactough/node-feedparser/subscription","commits_url":"https://api.github.com/repos/danmactough/node-feedparser/commits{/sha}","git_commits_url":"https://api.github.com/repos/danmactough/node-feedparser/git/commits{/sha}","comments_url":"https://api.github.com/repos/danmactough/node-feedparser/comments{/number}","issue_comment_url":"https://api.github.com/repos/danmactough/node-feedparser/issues/comments/{number}","contents_url":"https://api.github.com/repos/danmactough/node-feedparser/contents/{+path}","compare_url":"https://api.github.com/repos/danmactough/node-feedparser/compare/{base}...{head}","merges_url":"https://api.github.com/repos/danmactough/node-feedparser/merges","archive_url":"https://api.github.com/repos/danmactough/node-feedparser/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/danmactough/node-feedparser/downloads","issues_url":"https://api.github.com/repos/danmactough/node-feedparser/issues{/number}","pulls_url":"https://api.github.com/repos/danmactough/node-feedparser/pulls{/number}","milestones_url":"https://api.github.com/repos/danmactough/node-feedparser/milestones{/number}","notifications_url":"https://api.github.com/repos/danmactough/node-feedparser/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/danmactough/node-feedparser/labels{/name}","releases_url":"https://api.github.com/repos/danmactough/node-feedparser/releases{/id}","created_at":"2011-09-15T14:36:25Z","updated_at":"2014-07-22T15:05:58Z","pushed_at":"2014-07-22T18:16:39Z","git_url":"git://github.com/danmactough/node-feedparser.git","ssh_url":"git@github.com:danmactough/node-feedparser.git","clone_url":"https://github.com/danmactough/node-feedparser.git","svn_url":"https://github.com/danmactough/node-feedparser","homepage":"","size":1193,"stargazers_count":568,"watchers_count":568,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":81,"mirror_url":null,"open_issues_count":9,"forks":81,"open_issues":9,"watchers":568,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119"},"html":{"href":"https://github.com/danmactough/node-feedparser/pull/119"},"issue":{"href":"https://api.github.com/repos/danmactough/node-feedparser/issues/119"},"comments":{"href":"https://api.github.com/repos/danmactough/node-feedparser/issues/119/comments"},"review_comments":{"href":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119/comments"},"review_comment":{"href":"https://api.github.com/repos/danmactough/node-feedparser/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/danmactough/node-feedparser/pulls/119/commits"},"statuses":{"href":"https://api.github.com/repos/danmactough/node-feedparser/statuses/ce9d04e32c078239ef2583f0104e7553dcd26507"}}}}