actions-on-google/actions-on-google-nodejs

NodeJS crash on reportState JSON.parse()

JorgeLosadaM opened this issue · 8 comments

Version:

2.10.0 (Despite the version we use is older, the bug is not patched in recent versions)

Node Version: 6.13.0

Description:

When a html response is feeded to the method reportState as apiResponse in the event 'end', the function JSON.parse(apiResponse) crashes node with this output:

Some names are masked for client privacy reasons

SyntaxError: Unexpected token < in JSON at position 0
Jun 27 11:51:11 a***************d node[6801]: at Object.parse (native)
Jun 27 11:51:11 a***************d node[6801]: at IncomingMessage.res.on (/home/*****/H**********r/node_modules/actions-on-google/dist/service/smarthome/smarthome.js:51:50)
Jun 27 11:51:11 a***************d node[6801]: at emitNone (events.js:91:20)
Jun 27 11:51:11 a***************d node[6801]: at IncomingMessage.emit (events.js:185:7)
Jun 27 11:51:11 a***************d node[6801]: at endReadableNT (_stream_readable.js:974:12)
Jun 27 11:51:11 a***************d node[6801]: at _combinedTickCallback (internal/process/next_tick.js:80:11)
Jun 27 11:51:11 a***************d node[6801]: at process._tickDomainCallback (internal/process/next_tick.js:128:9)
Jun 27 11:51:11 a***************d node[6801]: /home/****/H***********r/node_modules/ask-sdk-v1adapter/dist/adapter.js:272
Jun 27 11:51:11 a***************d node[6801]: throw err;
Jun 27 11:51:11 a***************d node[6801]: ^

Conclusion:

The error can not be catched with a try/catch structure, so before doing JSON.parse(), apiResponse has to be validated to make sure it is a stringified JSON object.

You are sending an HTML response into the reportState method?

I think JorgeLosadaM meant the response to the reportState (which is sent from Google) contained a non-JSON response (probably HTML).

We are facing the same issue. From time to time, our node crashed because of this:

Unexpected token < in JSON at position 0 SyntaxError: Unexpected token < in JSON at position 0
at JSON.parse ()
at IncomingMessage.res.on (/****/node_modules/actions-on-google/dist/service/smarthome/smarthome.js:51:50)
at IncomingMessage.emit (events.js:203:15)
at endReadableNT (_stream_readable.js:1145:12)
at process._tickCallback (internal/process/next_tick.js:63:19)

Yes, sometimes the response to the reportState contained HTML which lead to node crashes.

We solved this issue temporarily with a fork of the repository until we could upgrade to node12, but for older versions the issue is still relevant.

Hi JorgeLosadaM,

Is it possible to share the fork with us?

Does the node version matter? We're using node 10.

We do not have de project forked anymore, but I strongly suggest you to upgrade node to at least node12 because it can handle the type of error this issue raises.
The problem with older versions is that it directly crashes the node instance, in node12 we catch the error with a simple try...catch structure and we ignore it.

I hope this helps.

Also if you want to create a fork yourself instead of updating node, i think the fix was changing the line 301 in src/service/smarthome/smarthome.ts to this one:
const apiResponseJson = JSON.parse(JSON.stringify(apiResponse))

Thanks JorgeLosadaM

The makeApiCall function creates an HTTP request to google's server, and in the POST headers, it does not specify the header "Accept: application/json". It seems that in (at least some) cases of error in Google's server, it returns back xml. Setting the accept header to "application/json" will likely fix this crash from occurring.

The second thing is that if JSON.parse throws an error, there is not any try/catch to catch the error.