Skip to content
This repository has been archived by the owner on May 28, 2024. It is now read-only.

NodeJS crash on reportState JSON.parse() #411

Open
JorgeLosadaM opened this issue Nov 26, 2020 · 8 comments
Open

NodeJS crash on reportState JSON.parse() #411

JorgeLosadaM opened this issue Nov 26, 2020 · 8 comments

Comments

@JorgeLosadaM
Copy link

JorgeLosadaM commented Nov 26, 2020

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
Copy link
Member

Fleker commented Dec 2, 2020

You are sending an HTML response into the reportState method?

@miclee
Copy link

miclee commented Feb 24, 2021

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
Copy link
Author

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
Copy link

miclee commented Feb 25, 2021

Hi JorgeLosadaM,

Is it possible to share the fork with us?

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

@JorgeLosadaM
Copy link
Author

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
Copy link
Author

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
Copy link

miclee commented Feb 26, 2021

Thanks JorgeLosadaM

@jtut1731
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants