wrap json.parse in response with try,catch

clonn / slack-node-sdk

slack.com, slack, node sdk

MIT License

167 stars 32 forks source link

wrap json.parse in response with try,catch #29

Closed guyklainer closed 8 years ago

guyklainer commented 8 years ago

When the response is too large, json.parse is throwing error of too long message.

kevinburkeshyp commented 8 years ago

Uh oh... looks like the files in lib and src have gotten out of sync (this fix has been applied to the file in src). Investigating...

guyklainer commented 8 years ago

Yes.. sorry.. i've made the fix in the lib folder instead of in the src folder.. To make another pull request with the fix in the src folder?

ultimatezen commented 8 years ago

Hi, curious if there have been any updates on this? Did you find out why lib and src got out of sync? We seem to be getting some bad responses from Slack lately that have been causing some trouble.

guyklainer commented 8 years ago

hi.. can you make the src and lib be synced again..?? this bug is killing me!!! waste all day on this JSON.parse issue again..........

kevinburkeshyp commented 8 years ago

Hi Guy - the correct code should (hopefully) be in the JS folder now, along with a test. Can you give it a try ?

kevinburkeshyp commented 8 years ago

Not sure I have the power to publish a new version to npm but it's in master

guyklainer commented 8 years ago

right, So who can publish it?

kevinburkeshyp commented 8 years ago

I just pushed 0.1.7 which shouldn't crash the server any more. hope that works!

guyklainer commented 8 years ago

in lib it looks good.. but in src the try,catch is missing now.. next build will overwrite the fix in the lib...

kevinburkeshyp commented 8 years ago

I'm a little confused - I just installed 0.1.7 and I see it in both places.

ag -B 1 -A 4 JSON.parse src lib src/lib/slack.seed.coffee 91- try 92: parsedResponse = JSON.parse(response) 93- catch err 94- err = new Error "Couldn't parse Slack API response as JSON:\n#{response}" 95- return callback?(err) 96-

lib/lib/slack.seed.js 110- try { 111: parsedResponse = JSON.parse(response); 112- } catch (_error) { 113- err = _error; 114- err = new Error("Couldn't parse Slack API response as JSON:\n"

response); 115- return typeof callback === "function" ? callback(err) : void 0;

On Mon, Dec 14, 2015 at 11:58 PM, Guy Klainer notifications@github.com wrote:

in lib it looks good.. but in src the try,catch is missing now.. next build will overwrite the fix in the lib...

— Reply to this email directly or view it on GitHub https://github.com/clonn/slack-node-sdk/pull/29#issuecomment-164678261.

kevin

guyklainer commented 8 years ago

oops.. sorry.. you are right.. i guess it was cache or something in my webstorm..

Thanks!