search

serviejs / popsicle

Simple HTTP requests for node and the browser
MIT License
246 stars 19 forks source link

Different behaviors of Body.arrayBuffer() between Nodejs and Browser #141

Closed sangelastro closed 4 years ago

sangelastro commented 4 years ago

Hi,

the arrayBuffer() method of the Body class behaves differently depending on it is in nodejs or in browser version, even though both return an ArrayBuffer. Let's suppose there is an endpoint that is expected to return an octet-stream media type, e.g., file stream result, memory stream (c#-like types, based on a byte array).

In the following, I report the issue/problem I encountered after calling the fetch function that accepts an 'octet-stream':

While in nodejs the arrayBuffer() method extracts from the rawBody (in common.js) a stream object, and the latter is properly converted into an ArrayBuffer by the node.js function streamToBuffer(stream) (and I'd say correctly converted as it matches exactly with the content of the object that the endpoint returned), in the browser version something changes. In fact, the "rawBody" object is an utf8 string conversion (with loss of information) of the object the endpoint returned. Then, the streamToArrayBuffer(rawBody) in browser.js converts forcibly (I may be wrong saying that) the string rawBody into an array buffer. The content of the returned object (in this case) is totally wrong and does not match with the one the endpoint returned.

I kindly ask you either if the above problem can represent a bug/issue, or some hints on how to solve this problem.

In the image below you can see the value of the rawBody of the response in the browser version of the library. The byte array (c#-like stream type) is converted into a string. image

Thanks in advance Sergio A.

blakeembrey commented 4 years ago

This is a tricky one and may require replacing XHR with fetch, which can probably be done nowadays since XHR was around to support old IE versions. I’ll take a look and see if there’s something I can do regardless, but this happens because with XHR you need to specify “type” before you get a response and we’re only getting strings today (streaming doesn’t currently work in the browser either). I expect to resolve this over the weekend since it’s a larger problem with using XHR and needs thought about older browsers.

sangelastro commented 4 years ago

Hi,

Honestly, I did not get what you mean by saying "replacing XHR with fetch". Also, how can I specify the "type" with XHR before getting the response? The last question is the following:

Thanks in advance for your availability, I am looking forward to getting fixed this bug.

Cordiality Sergio A.

blakeembrey commented 4 years ago

I've released a new minor version that supports arraybuffer natively (loses support for IE < 10), let me know if that works for you!

What I meant was only internal comments, sorry! For complete support of streaming and other features available in node.js, we'd need to switch to fetch in browser environments (entirely transparent to you). The only downside is that there's no fetch on IE, but that's likely a non-issue since there's other non-IE dependencies that popsicle requires today.

sangelastro commented 4 years ago

Hi,

No, It still does not work. Checking out your latest commits I noticed that only popsicle and popsicle-transport-xhr package.json have been modified. Is it correct?

blakeembrey commented 4 years ago

Yes.

blakeembrey commented 4 years ago

Can you describe more about what doesn't work? I tested it on some trivial examples and it appeared to work. The log you did above should be enough to help me understand.

sangelastro commented 4 years ago

[SOLVED]

Hi,

I have been trying hard to grab the bug/issue that was leading to this unexpected behavior, and finally I got it! I am so pleased to inform you that it now works smoothly. The $rawBody is correctly typed as an arrayBuffer, when the expected return value of my endpoint is an octet-stream media type, as you can see in the following:

image

I am so regret to notify you that libraries for browser stream handling affected the expected behavior. So, it's their fault.

T.hanks for your helping and availability

Sergio

blakeembrey commented 4 years ago

Perfect, thanks for the update! I'll close this out as resolved and look into fetch in the browser for the next major version. You shouldn't see any difference, but one of the benefits of fetch is that it can support streaming while XHR doesn't. For buffered use-cases the behavior would be identical.