HTTP server fails if body is a String object

[tve]

Moddable SDK version: 4.3 Target device: esp32

Description When a "prepareResponse" callback of HTTP server returns a body consisting of a String object (as opposed to a string) the server throws an exception causing the request to be aborted. The reason for this is a check against typeof this.body:

count = ("string" === typeof this.body) ? this.body.length : this.body.byteLength;  //@@ utf-8 hell

https://github.com/Moddable-OpenSource/moddable/blob/public/modules/network/http/http.js#L687

Steps to Reproduce

1. Create a handler that returns something like {status: 200, body: new String('foo')}

Note that this is not the only instance of this erroneous check in this module...

[phoddie]

I'm not sure this is a bug. The API accepts a string or an object. If it is an object, it needs to be a buffer/view.

What would you propose to change here?

[tve]

The docs state "If the body is a String or ArrayBuffer, it is the complete response." I am literally returning a String. Also, I believe it is fair to say that generally JS developers view string and String to behave the same (e.g. note the use of the String class in headersComplete: I assume the callback gets a string not a String...). If you do not want to support String then I suggest fixing the docs to say "string (not a String object)".

[tve]

NB: in light of #1271 maybe the best is to either not support strings or to consistently perform a TextEncoder transformation right away.

[phoddie]

I understand it is subtle, but there is a difference between a string and a boxed string (string object). When they behave the same, it is generally because the implementation cals toString() on the argument. That's not an option here because an object is interpreted as a buffer. I don't see a strong need for a code change. The caller can easily pass a string instead of a string object.

The ECMA-419 socket and network protocol implementations (e.g. http) don't accept strings, only buffers. That avoids the problem.

[tve]

I guess it comes down to overall developer experience. If the docs had been accurate it would have saved me a bunch of time and aggravation. If the code had lived up to the docs, ditto. I know the difference between String and string but it's not something the vast majority of JS APIs slap me in the face with.

[phoddie]

I think this might be a simple solution. Around line 678, change:

    this.body = response.body;

...to...

    this.body = (response.body instanceof String) ? response.body.valueOf() : response.body;

[phoddie]

it's not something the vast majority of JS APIs slap me in the face with.

This API accepts a string or an object. In that case, the distinction matters. I'm sorry you don't like it, but it is what it is here.

[tve]

I can confirm that the code change fixes the issue for me.

[phoddie]

Thanks. I'll commit that.

[phoddie]

Closing (fixed).