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

Node.js client library for Actions on Google
https://actions-on-google.github.io/actions-on-google-nodejs
Apache License 2.0
899 stars 193 forks source link

NodeJS crash on reportState JSON.parse() #411

Open JorgeLosadaM opened 4 years ago

JorgeLosadaM commented 4 years ago

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.

Fleker commented 3 years ago

You are sending an HTML response into the reportState method?

miclee commented 3 years ago

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)

JorgeLosadaM commented 3 years ago

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.

miclee commented 3 years ago

Hi JorgeLosadaM,

Is it possible to share the fork with us?

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

JorgeLosadaM commented 3 years ago

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.

JorgeLosadaM commented 3 years ago

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))

miclee commented 3 years ago

Thanks JorgeLosadaM

jtut1731 commented 3 years ago

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.