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

embedded:network/http/client hangs if server sends no response body #1219

Closed
tve opened this issue Sep 23, 2023 · 2 comments
Closed

embedded:network/http/client hangs if server sends no response body #1219

tve opened this issue Sep 23, 2023 · 2 comments

Comments

@tve
Copy link
Contributor

tve commented Sep 23, 2023

Build environment: Linux
Moddable SDK version: idf-v5
Target device: esp32-c3

Description
If the server response does not include a response body then the client hangs after the onHeaders callback. There is no onDone callback and subsequent requests are never processed. A typical case is a POST that results in a 204 "No Content" status.

Steps to Reproduce

  1. Run the following variant of examples/io/tcp/httpclient:
/*
 * Copyright (c) 2021-2022  Moddable Tech, Inc.
 *
 *   This file is part of the Moddable SDK.
 * 
 *   This work is licensed under the
 *       Creative Commons Attribution 4.0 International License.
 *   To view a copy of this license, visit
 *       <http://creativecommons.org/licenses/by/4.0>.
 *   or send a letter to Creative Commons, PO Box 1866,
 *   Mountain View, CA 94042, USA.
 *
 */

import TextDecoder from "text/decoder"

const HTTPClient = device.network.http.io;
const http = new HTTPClient({ 
	...device.network.http,
	host: "httpbin.org"
});
for (let i = 0; i < 3; i++) {
	http.request({
		path: i == 1 ? `/status/204` : `/get?foo=bar`,
		headers: new Map([
			["date", Date()],
			["user-agent", "ecma-419 test"]
		]),
		onHeaders(status, headers) {
			trace(`Status ${status}, Content-Type ${headers.get("content-type")}\n`);
			this.decoder = new TextDecoder;
		},
		onReadable(count) {
			trace(this.decoder.decode(this.read(count), {stream: true})); 
		},
		onDone() {
			trace(`\n\n **DONE ${i} **\n\n`);
		}
	});
}
  1. You should see
Status 200, Content-Type application/json
{
  "args": {
    "foo": "bar"
  }, 
  "headers": {
    "Date": "Sat Sep 23 2023 13:36:19 GMT-0700", 
    "Host": "httpbin.org", 
    "User-Agent": "ecma-419 test", 
    "X-Amzn-Trace-Id": "Root=1-650f4c43-49b9506544dcfcce3fa4451d"
  }, 
  "origin": "47.151.203.177", 
  "url": "http://httpbin.org/get?foo=bar"
}


 **DONE 0 **

Status 204, Content-Type text/html; charset=utf-8
  1. After a minute or so (prob. when the server closes the socket) you should see:
 **DONE 1 **

Expected behavior
I expected to see **DONE 1 ** immediately and I expected the third request to succeed.

@phoddie
Copy link
Collaborator

phoddie commented Sep 24, 2023

HTTP 204 is a special case where the response body is always empty. The usual Content-Length field is not included with a 0 because the status implies it. I think HTTP 205 is the same.

Changing these lines in httpclient.js...

if (undefined !== this.#chunk)
this.#remaining = undefined; // ignore content-length if chunked
else if (undefined === this.#remaining)
this.#remaining = Infinity;
this.#current.onHeaders?.call(this.#current.request, this.#status, this.#headers);
if (!this.#current) return; // closed in callback
this.#headers = undefined;
this.#state = "receiveBody";
this.#line = (undefined == this.#chunk) ? undefined : "";

...to the following, gives the expected result.

	if ((204 === this.#status) || (205 === this.#status))
		this.#remaining = 0;
	else if (undefined !== this.#chunk)
		this.#remaining = undefined;		// ignore content-length if chunked
	else if (undefined === this.#remaining)
		this.#remaining = Infinity;

	this.#current.onHeaders?.call(this.#current.request, this.#status, this.#headers);
	if (!this.#current) return;			// closed in callback

	this.#headers = undefined;
	this.#state = "receiveBody";
	this.#line = (undefined == this.#chunk) ? undefined : "";

	if (0 === this.#remaining)
		return void this.#done();

It looks like this might also help with the case where "Content-Length" is 0.

Does that work for you as well?

mkellner pushed a commit that referenced this issue Sep 26, 2023
@phoddie
Copy link
Collaborator

phoddie commented Sep 29, 2023

Fix committed. Closing. (If problem persists, please reopen.)

@phoddie phoddie closed this as completed Sep 29, 2023
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

2 participants