Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Handle case where res.body is undefined after making a request #2847

Closed
pcowgill opened this issue Jan 10, 2020 · 7 comments
Closed

Handle case where res.body is undefined after making a request #2847

pcowgill opened this issue Jan 10, 2020 · 7 comments
Labels
pkg:http-client Issues related only to ipfs-http-client

Comments

@pcowgill
Copy link
Contributor

It would be nice if you could add error handling for if res.body is undefined after making a ky request.

Like here, for instance:

https://github.com/ipfs/js-ipfs-http-client/blob/da9d17a38ce09d299e7180d489a56c1e276b4fb9/src/add/index.js#L43

That way in envs that need a polyfill or where a polyfill is used that doesn't implement the complete fetch spec, the error message will be more obvious.

Thanks!

@pcowgill
Copy link
Contributor Author

(Or as @hugomrdias and I discussed, handle the case without throwing an error by passing res through to toIterable and modifying the logic of that function to handle the body undefined case)

@pcowgill pcowgill changed the title Add error handling for if res.body is undefined after making a ky request Handle case where res.body is undefined after making a ky request Jan 21, 2020
@achingbrain achingbrain transferred this issue from ipfs-inactive/js-ipfs-http-client Mar 9, 2020
@achingbrain achingbrain added the pkg:http-client Issues related only to ipfs-http-client label Mar 9, 2020
@pcowgill pcowgill changed the title Handle case where res.body is undefined after making a ky request Handle case where res.body is undefined after making a request Mar 9, 2020
@pcowgill
Copy link
Contributor Author

pcowgill commented Mar 9, 2020

@hugomrdias Let me know if you're swayed by the argument that we can make this change and then undo it as soon as it lands upstream in React Native. Thanks! Also should we link to a tracking issue in React Native here or here?

@pcowgill
Copy link
Contributor Author

pcowgill commented Mar 9, 2020

For now I think this serves as the tracking issue in the React Native repo facebook/react-native#27741

@hugomrdias
Copy link
Member

Issues related to a incomplete spec impl will be handled outside http-client, add all those links to the rn track issue I made and we will figure it out

@pcowgill
Copy link
Contributor Author

Ah ok, fair enough. Will do!

I think the rationale for having it land here is that fetch is only a spec for the browser environment, so it's that package's job to make sure there's an alternate solution in node and React Native environments.

But I think there are plenty of valid counterpoints to that point, so we definitely can have it land outside that package.

Now that this is a monorepo, are you picturing it landing in another child package in this repo or in a separate repo? I assume the latter, but just confirming. Thanks!

@pcowgill
Copy link
Contributor Author

Responding to @qalqi's latest comments in the old github/fetch issue that will live on in the react-native-community/fetch issue. Responding here since the last comments specifically related to IPFS.

A) go-ipfs daemon and js-ipfs daemon must be same

What do you mean by this? This isn't necessarily the case. For instance, right now using dag.put from ipfs-http-client pointing at js-ipfs isn't working, but it is working when pointing at go-ipfs. #2825

B) Yeah I am using ipfs-http-client with version 42.0.0

Ah okay, good to know. Thanks! FYI a big PR is about to land in ipfs-http-client that removes the ky dependency, so things might change soon with the @next release.

All get requests are working with ipfs-http-client after changing fetch logic as mentioned above.

This is my experience as well - cat is working. The operations that aren't working for me are ones related to adding information to an IPFS daemon.

Put requests are all failing with error Current request is not a multipart request

Interesting, I haven't seen that specific error yet. Is this snippet the exact request that gave you that error? https://github.com/qalqi/react-native-ipfs-http-client/blob/fc713153239c7a99466b39a4a4890baca08b5ff5/rn-nodeify/App.js#L74-L83

So you're using block.put? So far on the upload side I have mostly focused on add and dag.put, so it's likely I would get the same error message you are.

Here is react-native sandbox with working get requests https://github.com/qalqi/react-native-ipfs-http-client/tree/primary/rn-nodeify

Perfect, thanks for sharing this.

@hugomrdias
Copy link
Member

Closing this in favor of #2813

Thank you

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:http-client Issues related only to ipfs-http-client
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants