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

Inconsistent response handling in v2 #40

Open
saibotsivad opened this issue Feb 24, 2022 · 1 comment
Open

Inconsistent response handling in v2 #40

saibotsivad opened this issue Feb 24, 2022 · 1 comment

Comments

@saibotsivad
Copy link

Consider a request to a server that successfully returns XML data (aka anything that is not JSON).

In the node version we have this flow:

  1. construct out as the concatenated data stream string
  2. since the header content-type does not include application/json so...
  3. the response data property is set to the out (aka the response body realized)
  4. the promise is resolved with that response (aka response.data is the response body XML string)

However, in the fetch version we have this flow:

  1. the header content-type does not include application/json so...
  2. try to parse the string from fetch.text() as JSON
  3. since the response is XML this throws
  4. set the error on the response
  5. the promise is rejected but the fetch.text() string is not available on the response
@saibotsivad
Copy link
Author

Well I may have spent too long staring at code and fried my brain, I see now !~tmp.indexOf('application/json') would be true in my example, so that flow result is incorrect. 🤦

The response is still inconsistent, but in a way that I can handle:

The node version resolves the promise with response.data being the fully realized response body XML string.

Whereas the fetch version simply resolves the promise with response, aka the response body is not yet fully realized.

My resolution to this was, in my code, to simply do:

let response = await httpie(url, options)
const bodyString = typeof response.data === 'string'
    ? response.data
    : await response.text()

I don't know what the "Right Thing" to do here would be, since the goal isn't full strict isomorphism between the two, but since you're waiting for the body stream to be realized in the Node version, it seems like it might be appropriate to do the same in the fetch version? Something like this, probably:

			if (!tmp || !~tmp.indexOf('application/json')) {
-				reply(rr);
+				rr.text().then(str => {
+					rr.data = str;
+					reply(rr);
+				});
			} else {

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

1 participant