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.

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

Jump to bottom

Unhandled exception while parsing response body #347

Closed
MaxMadylius opened this issue Dec 9, 2016 · 10 comments
Closed

Unhandled exception while parsing response body #347

MaxMadylius opened this issue Dec 9, 2016 · 10 comments
Labels
status: help wanted requesting help from the community type: bug bug in the library

Comments

@MaxMadylius
Copy link

Issue Summary

In some cases SendGrid returns html response. For example if we send email over 25 mb.
If response.body is not json it causes crash.

Technical details:

Unhandled exception occurs in sendgrid.js file line 95 and 110.

  • sendgrid-nodejs Version: 4.7.1
  • Node.js Version: 6.4.0
@thinkingserious thinkingserious added status: help wanted requesting help from the community type: bug bug in the library labels Dec 10, 2016
@thinkingserious
Copy link
Contributor

Thanks for the issue @MaxMadylius!

I've added this to our backlog for an investigation and fix.

@thinkingserious thinkingserious mentioned this issue Dec 10, 2016
Closed
@coreprocess
Copy link

this is serious, please fix!

@coreprocess
Copy link

You can simulate it easily with providing an invalid URL. This is what happens:

404 page not found
    ^

SyntaxError: Unexpected token p in JSON at position 4
    at JSON.parse (<anonymous>)
    at /home/niklas/Dev/bop-legaltech/node_modules/sendgrid/lib/sendgrid.js:110:42
    at IncomingMessage.<anonymous> (/home/niklas/Dev/bop-legaltech/node_modules/sendgrid-rest/lib/client.js:108:9)
    at emitNone (events.js:91:20)
    at IncomingMessage.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:974:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)

This should not crash, but rather should return an error to the callback!

@sgehly
Copy link

sgehly commented Dec 24, 2016

It seems that SendGrid is always expecting a JSON reply. Removing 110 altogether provides:

0|app      |    { statusCode: 404,
0|app      |      body: '<html>\r\n<head><title>404 Not Found</title></head>\r\n<body bgcolor="white">\r\n<center><h1>404 Not Found</h1></center>\r\n<hr><center>nginx</center>\r\n</body>\r\n</html>\r\n',
0|app      |      headers: 
0|app      |       { server: 'nginx',
0|app      |         date: 'Sat, 24 Dec 2016 05:16:55 GMT',
0|app      |         'content-type': 'text/html',
0|app      |         'content-length': '162',
0|app      |         connection: 'close' } } } { statusCode: 404,
0|app      |   body: '<html>\r\n<head><title>404 Not Found</title></head>\r\n<body bgcolor="white">\r\n<center><h1>404 Not Found</h1></center>\r\n<hr><center>nginx</center>\r\n</body>\r\n</html>\r\n',

...which would obviously fail the parser.

@thinkingserious
Copy link
Contributor

Thanks for the votes @Core-Process, @sgehly!

I have added them to this item in our backlog.

@danconnell
Copy link

danconnell commented Mar 28, 2017 •
edited
Loading

Just had this happen. Not sure what the exact payload was that triggered it, but it looks like we got a HTML response back which resulted in:

SyntaxError: Unexpected token < in JSON at position 0
  ?, in Object.parse
  File "/app/node_modules/sendgrid/lib/sendgrid.js", line 95, col 46, in null.<anonymous>
    response.body = response.body ? JSON.parse(response.body) : response.body;
  File "/app/node_modules/sendgrid-rest/lib/client.js", line 108, col 9, in IncomingMessage.<anonymous>
    callback(response)
  File "events.js", line 91, col 20, in emitNone
  File "events.js", line 185, col 7, in IncomingMessage.emit
  File "_stream_readable.js", line 974, col 12, in endReadableNT
  File "internal/process/next_tick.js", line 74, col 11, in _combinedTickCallback
  File "internal/process/next_tick.js", line 122, col 9, in process._tickDomainCallback

On v4.8.3 of this package.

@SendGridDX
Copy link
Collaborator

Thanks for helping us verify the issue @danconnell! Also, I've added your vote to this issue in our backlog.

@thinkingserious
Copy link
Contributor

Hello Everyone,

I believe v4.9 fixed this issue. Please let me know if you continue to experience issues after upgrading. Thanks!

@maximivanov
Copy link

Just got this in 5.1.1. Reason was I copypasted endpoint url from docs and forgot to prepend request path with "/v3". It would be nice to get proper 404 and not SyntaxError in this case.

@thinkingserious
Copy link
Contributor

Thanks for the heads up @maximivanov. I think this issue will be fixed with this PR: #378

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: help wanted requesting help from the community type: bug bug in the library
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@thinkingserious @coreprocess @maximivanov @danconnell @sgehly @SendGridDX @MaxMadylius