Skip to content
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.

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

Handling syntax error while parsing HTML response #18

Merged
merged 2 commits into from Nov 7, 2017

Conversation

@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Sep 19, 2017

@mchaudhary @mostlyjason, This PR is to resolve the issue #17.

In this PR, I have put an if condition to check if the body contains some data and not undefined and also parse the body when the response code is 200 OK. When there is some error returned from the server in HTML form in the body and when it goes to parse, it throws a syntax error. So I am checking both the body and the response code to parse the body further.

Please review.

self.emit('log', result);
if (callback) {
callback(null, result);
if(body && res.statusCode.toString() === '200'){

This comment has been minimized.

@mchaudhary

mchaudhary Oct 31, 2017

We should check for non 200 code and log the message.

This comment has been minimized.

@Shwetajain148

Shwetajain148 Nov 2, 2017
Author

@mchaudhary, Here if the returned response code is 200 then the body contains {"response" : "ok"} which then get parsed normally. Now If the returned response code is other than 200 and in HTML format as I created a scenario in which I was getting status code 503 "Service Unavailable" then the library breaks to parse the HTML format.

I am not able to check with other server error codes like 500, 501 and 504 etc to see in what format they are returning the response and other error codes in 400 series like I tested with 403(wrong token) and 404(Not found) but in both the cases the control did not come to parse the body object and there was nothing in the body.

In the else part, I am also printing the error code with the error message.

Shwetajain148
@mchaudhary mchaudhary merged commit 361c291 into loggly:master Nov 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.