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

fetch incorrectly handling response of content-length 0 #1080

Closed
NW-Lab opened this issue Apr 6, 2023 · 12 comments
Closed

fetch incorrectly handling response of content-length 0 #1080

NW-Lab opened this issue Apr 6, 2023 · 12 comments

Comments

@NW-Lab
Copy link
Contributor

NW-Lab commented Apr 6, 2023

Build environment: Windows + Node-RED MCU Plugin
Target device: ESP32(m5stick_cplus)

Description
http request may stop at "c:\pjt\moddable\examples\io\tcp\fetch\fetch.js (162) # Break: fromArrayBuffer: argument is no ArrayBuffer instance!"

Node-RED Flow

[{"id":"d08ce3b97bd479ba","type":"tab","label":"Ambient.io","disabled":false,"info":"","env":[],"_mcu":{"mcu":true}},{"id":"934195a49ffabf80","type":"inject","z":"d08ce3b97bd479ba","name":"","props":[{"p":"payload"},{"p":"topic","vt":"str"}],"repeat":"30","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"","payloadType":"date","_mcu":{"mcu":true},"x":270,"y":260,"wires":[["44371d2345dc887c"]]},{"id":"3d973510489ea316","type":"http request","z":"d08ce3b97bd479ba","name":"","method":"POST","ret":"obj","paytoqs":"ignore","url":"http://ambidata.io/api/v2/channels/*/data","tls":"","persist":false,"proxy":"","insecureHTTPParser":false,"authType":"","senderr":false,"headers":[{"keyType":"other","keyValue":"Content-Type","valueType":"other","valueValue":"application/json"}],"_mcu":{"mcu":true},"credentials":{},"x":770,"y":260,"wires":[["733252737ac78dd8"]]},{"id":"999732051de18b98","type":"template","z":"d08ce3b97bd479ba","name":"","field":"payload","fieldType":"msg","format":"handlebars","syntax":"mustache","template":"{\"writeKey\":\"*****\",\"d1\":{{payload.thermometer.temperature}},\"d2\":{{payload.hygrometer.humidity}}}\n","output":"json","_mcu":{"mcu":true},"x":600,"y":260,"wires":[["3d973510489ea316","c187c1861f7112da"]]},{"id":"9d7c6f07286f609a","type":"ui_text","z":"d08ce3b97bd479ba","group":"99181448cf6a5df3","order":0,"width":0,"height":0,"name":"","label":"Temperature","format":"{{msg.payload}}","layout":"row-spread","className":"","_mcu":{"mcu":true},"x":770,"y":340,"wires":[]},{"id":"23ef658ac174b530","type":"ui_text","z":"d08ce3b97bd479ba","group":"99181448cf6a5df3","order":1,"width":0,"height":0,"name":"","label":"humidity","format":"{{msg.payload}}","layout":"row-spread","className":"","_mcu":{"mcu":true},"x":760,"y":400,"wires":[]},{"id":"0135e35590a828a5","type":"function","z":"d08ce3b97bd479ba","name":"function 2","func":"const temp = msg.payload.thermometer.temperature;\nmsg.payload =temp.toFixed(2);\nreturn msg;","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"_mcu":{"mcu":true},"x":600,"y":340,"wires":[["9d7c6f07286f609a"]]},{"id":"cf71fcb8a4375c1c","type":"function","z":"d08ce3b97bd479ba","name":"function 3","func":"const humi = Math.round(msg.payload.hygrometer.humidity);\nmsg.payload=humi;\nreturn msg;","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"_mcu":{"mcu":true},"x":600,"y":400,"wires":[["23ef658ac174b530"]]},{"id":"733252737ac78dd8","type":"debug","z":"d08ce3b97bd479ba","name":"debug 4","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","statusVal":"","statusType":"auto","_mcu":{"mcu":true},"x":940,"y":260,"wires":[]},{"id":"44371d2345dc887c","type":"sensor","z":"d08ce3b97bd479ba","name":"","platform":"","module":"embedded:sensor/Humidity-Temperature/SHT3x","options":{"sensor":{"io":"I2C","bus":"default","address":"0x44"}},"configuration":"{}","moddable_manifest":{"include":["$(MODDABLE)/modules/drivers/sensors/sht3x/manifest.json"]},"_mcu":{"mcu":true},"x":450,"y":260,"wires":[["999732051de18b98","0135e35590a828a5","cf71fcb8a4375c1c"]]},{"id":"c187c1861f7112da","type":"debug","z":"d08ce3b97bd479ba","name":"debug 5","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","statusVal":"","statusType":"auto","_mcu":{"mcu":true},"x":800,"y":200,"wires":[]},{"id":"99181448cf6a5df3","type":"ui_group","name":"tb","tab":"3db2d4310e9d80cc","order":1,"disp":false,"width":"6","collapse":false},{"id":"3db2d4310e9d80cc","type":"ui_tab","name":"home","icon":"dashboard","disabled":false,"hidden":false}]

Images
image
img1

Other information
The probability of success and failure is about 50%

I'm sorry if I'm using it wrong.

@phoddie
Copy link
Collaborator

phoddie commented Apr 6, 2023

I'll try to reproduce this later tonight. Thank you for including your test flow.

@phoddie
Copy link
Collaborator

phoddie commented Apr 6, 2023

I don't think I can reproduce this without the writeKey. Posting without the key causes the server to return the string "Not Found" instead of a JSON body. If possible, please share a temporary key via email. If not, perhaps there is another way to reproduce this?

If it helps, I created this simplified flow that does not use the sensor or dashboard, to simplify my debugging.

[
    {
        "id": "d08ce3b97bd479ba",
        "type": "tab",
        "label": "Ambient.io",
        "disabled": false,
        "info": "",
        "env": []
    },
    {
        "id": "934195a49ffabf80",
        "type": "inject",
        "z": "d08ce3b97bd479ba",
        "name": "",
        "props": [
            {
                "p": "payload"
            }
        ],
        "repeat": "30",
        "crontab": "",
        "once": true,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "{\"thermometer\":{\"temperature\":20},\"hygrometer\":{\"humidity\":30}}",
        "payloadType": "json",
        "x": 250,
        "y": 260,
        "wires": [
            [
                "999732051de18b98"
            ]
        ]
    },
    {
        "id": "3d973510489ea316",
        "type": "http request",
        "z": "d08ce3b97bd479ba",
        "name": "",
        "method": "POST",
        "ret": "obj",
        "paytoqs": "ignore",
        "url": "http://ambidata.io/api/v2/channels/*/data",
        "tls": "",
        "persist": false,
        "proxy": "",
        "insecureHTTPParser": false,
        "authType": "",
        "senderr": false,
        "headers": [
            {
                "keyType": "other",
                "keyValue": "Content-Type",
                "valueType": "other",
                "valueValue": "application/json"
            }
        ],
        "x": 770,
        "y": 260,
        "wires": [
            [
                "733252737ac78dd8"
            ]
        ]
    },
    {
        "id": "999732051de18b98",
        "type": "template",
        "z": "d08ce3b97bd479ba",
        "name": "",
        "field": "payload",
        "fieldType": "msg",
        "format": "handlebars",
        "syntax": "mustache",
        "template": "{\"writeKey\":\"*****\",\"d1\":{{payload.thermometer.temperature}},\"d2\":{{payload.hygrometer.humidity}}}",
        "output": "json",
        "x": 500,
        "y": 260,
        "wires": [
            [
                "3d973510489ea316",
                "c187c1861f7112da"
            ]
        ]
    },
    {
        "id": "733252737ac78dd8",
        "type": "debug",
        "z": "d08ce3b97bd479ba",
        "name": "debug 4",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "false",
        "statusVal": "",
        "statusType": "auto",
        "x": 940,
        "y": 260,
        "wires": []
    },
    {
        "id": "c187c1861f7112da",
        "type": "debug",
        "z": "d08ce3b97bd479ba",
        "name": "debug 5",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "false",
        "statusVal": "",
        "statusType": "auto",
        "x": 800,
        "y": 200,
        "wires": []
    }
]

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Apr 6, 2023

Thank you for the sample flow.
I put the channel No. and writeKey of the sample.
(I closed the ISSUE, so I deleted writeKey and channel No.)

[
    {
        "id": "c5fcfc5d2cb51ebf",
        "type": "tab",
        "label": "Ambient.io",
        "disabled": false,
        "info": "",
        "env": [],
        "_mcu": {
            "mcu": true
        }
    },
    {
        "id": "cf6b27d8d91c3594",
        "type": "inject",
        "z": "c5fcfc5d2cb51ebf",
        "name": "",
        "props": [
            {
                "p": "payload"
            }
        ],
        "repeat": "30",
        "crontab": "",
        "once": true,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "{\"thermometer\":{\"temperature\":20},\"hygrometer\":{\"humidity\":30}}",
        "payloadType": "json",
        "_mcu": {
            "mcu": true
        },
        "x": 250,
        "y": 260,
        "wires": [
            [
                "c1246d87f03f574b"
            ]
        ]
    },
    {
        "id": "12594449713b3e27",
        "type": "http request",
        "z": "c5fcfc5d2cb51ebf",
        "name": "",
        "method": "POST",
        "ret": "obj",
        "paytoqs": "ignore",
        "url": "http://ambidata.io/api/v2/channels/*/data",
        "tls": "",
        "persist": false,
        "proxy": "",
        "insecureHTTPParser": false,
        "authType": "",
        "senderr": false,
        "headers": [
            {
                "keyType": "other",
                "keyValue": "Content-Type",
                "valueType": "other",
                "valueValue": "application/json"
            }
        ],
        "_mcu": {
            "mcu": true
        },
        "x": 770,
        "y": 260,
        "wires": [
            [
                "5d1850cd9ccc52d3"
            ]
        ]
    },
    {
        "id": "c1246d87f03f574b",
        "type": "template",
        "z": "c5fcfc5d2cb51ebf",
        "name": "",
        "field": "payload",
        "fieldType": "msg",
        "format": "handlebars",
        "syntax": "mustache",
        "template": "{\"writeKey\":\"*\",\"d1\":{{payload.thermometer.temperature}},\"d2\":{{payload.hygrometer.humidity}}}",
        "output": "json",
        "_mcu": {
            "mcu": true
        },
        "x": 500,
        "y": 260,
        "wires": [
            [
                "12594449713b3e27",
                "89c4cc64a101a963"
            ]
        ]
    },
    {
        "id": "5d1850cd9ccc52d3",
        "type": "debug",
        "z": "c5fcfc5d2cb51ebf",
        "name": "debug 4",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "false",
        "statusVal": "",
        "statusType": "auto",
        "_mcu": {
            "mcu": true
        },
        "x": 940,
        "y": 260,
        "wires": []
    },
    {
        "id": "89c4cc64a101a963",
        "type": "debug",
        "z": "c5fcfc5d2cb51ebf",
        "name": "debug 5",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "false",
        "statusVal": "",
        "statusType": "auto",
        "_mcu": {
            "mcu": true
        },
        "x": 800,
        "y": 200,
        "wires": []
    }
]

Please check.
You can see the data you entered at this URL. https://ambidata.io/bd/board.html?id=58402
It is an IoT visualization service that is often used in Japan.
Thank you.

@phoddie
Copy link
Collaborator

phoddie commented Apr 6, 2023

With the updated flow, I'm able to reproduce the error you report. What I see is that the HTTP server responds as follows:

}HTTP/1.1 200 OK
x-powered-by: Express
access-control-allow-origin: *
date: Thu, 06 Apr 2023 04:08:09 GMT
connection: close
content-length: 0

It looks like the 0 content-length response is not being handled correctly. The body is null which fails when passed to JSON.parse. I need to look into what fetch and the Node-RED HTTP Response node do in this situation, to match that behavior.

Very nice find. Thank you!

@phoddie
Copy link
Collaborator

phoddie commented Apr 6, 2023

The behavior seems to be that fetch throws an exception when the JSON.parse fails on an empty buffer. The Node-RED HTTP Response node catches the exception and outputs a message with the payload set to an empty string. Strangely, the Node-RED HTTP Response node does not trigger any Catch nodes so there's no way for a flow to detect the error.

I made some changes to match the fetch and Node-RED behaviors. I will review them tomorrow and then commit.

@tve
Copy link
Contributor

tve commented Apr 6, 2023

I suppose technically a 200 response has a body which here happens to be of length zero, i.e. "" (empty string), while a 204 response has no body and thus null might be appropriate?

@phoddie
Copy link
Collaborator

phoddie commented Apr 6, 2023

@tve – Thanks for the thoughts. This particular null is internal to the fetch implementation, so it can do whatever it likes as long as the observable result matches the standard. In my tests, it appears that status 200 with Content-Length of 0 generates:

  • response.json() - throws (because an empty string is not valid JSON)
  • response.text() - empty string
  • response.arrayBuffer() - zero length ArrayBuffer

For status 204, fetch seems to specify a null as you suggest, though the details are a tangle:

21: If response is not a network error and either request’s method is HEAD or CONNECT, or internalResponse’s status is a null body status, set internalResponse’s body to null and disregard any enqueuing toward it (if any).

And:

  1. If response’s status is a null body status, then throw a TypeError

Taking those two together, I think that means that response.json/text/arrayBuffer() for a 204 should throw. But, I would really want to test that reading before committing to an implementation.

@phoddie phoddie changed the title I wonder if it's a bug in tcp.fetch.js. fetch incorrectly handling response of content-length 0 Apr 6, 2023
phoddie added a commit to phoddie/node-red-mcu that referenced this issue Apr 6, 2023
@phoddie
Copy link
Collaborator

phoddie commented Apr 7, 2023

The original issue reported should now be resolved. Note that it requires updating the Moddable SDK and Node-RED MCU.

There is likely work to be done for conformance with "null body status." That should be done as a part of a more rigorous conformance testing effort around the fetch implementation.

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Apr 7, 2023

The "fetch.js" error no longer appears, but breaks with the message "#break :parse:1: invalid value".
image
am i using this wrong?
The sample is earlier.

@phoddie
Copy link
Collaborator

phoddie commented Apr 7, 2023

Your HTTP request node is configured to parse the response as JSON:

image

The response is not valid JSON, so there is an exception throw by the JavaScript JSON.parse function. That's the JavaScript language specified behavior. If you are running with the debugger, that exception will break in the debugger. If you then choose "Go" in the debugger, execution continues as normal.

You have some options to avoid the break in the debugger:

  1. Run with xsbug-log instead of xsbug. xsbug-log traces console output but does not stop on breakpoints. This is closest to what you experience when running with the Node-RED Editor. In fact, in the Node-RED console, you see the parse error traces also:
7 Apr 06:53:14 - [warn] [http request:12594449713b3e27] JSON parse error
  1. Change your HTTP request so it does not try to parse the result as JSON:
    image
  2. Disable "break on exception" in xsbug preferences
    image

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Apr 8, 2023

Thank you for your advice.

In the properties of the "http request" node, "Tip: If the JSON parse fails the fetched string is returned as-is."

Even with Node-RED on the PC, I confirmed that [warn] is displayed although the execution does not stop.

@phoddie
Copy link
Collaborator

phoddie commented Apr 8, 2023

Thank you for the reminder about the "tip". Actually, I had not implemented that. I just committed a fix for it (it doesn't matter for your flow).

I'm going to close this issue out as resolved. If there are follow-on issues, please open a separate issue.

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

No branches or pull requests

3 participants