HTTP client factory: Invalid URL

[acostalima]

• Version:

js-ipfs version: 0.49.1-84cfa553ffc717d5d8bf94fdf6a306f182c9aee4
interface-ipfs-core version: ^0.139.1
ipfs-http-client version: ^46.0.1
Repo version: 8
System version: x64/darwin
Node.js version: v14.7.0
Commit: 84cfa55

• Platform:

MacOS Catalina 10.15.6

Darwin acostalima 19.6.0 Darwin Kernel Version 19.6.0: Thu Jun 18 20:49:00 PDT 2020; root:xnu-6153.141.1~1/RELEASE_X86_64 x86_64

React Native v0.63.2 (tracking RN support at #2813)

• Subsystem:

HTTP client

Severity:

High (?)

Description:

Call the HTTP client factory with a valid URL to create a new client. The factory throws with TypeError (Invalid URL).

Steps to reproduce the error:

import createHttpClient from 'ipfs-http-client';

createHttpClient({ url: 'http://localhost:5002' }); // throws TypeError 'Invalid URL'

I've tracked down the issue to:

js-ipfs/packages/ipfs-http-client/src/lib/core.js

Line 28 in 4b8021d

const url = new URL(options.url)

where options.url is undefined.

In the client's constructor, input is being normalized twice:

js-ipfs/packages/ipfs-http-client/src/lib/core.js

Line 116 in 4b8021d

const opts = normalizeInput(options)

js-ipfs/packages/ipfs-http-client/src/lib/core.js

Line 120 in 4b8021d

base: normalizeInput(opts.url).toString(),

where opts.url is already a URL object from the first pass. URLs are not being handled by the normalization logic, so options.url ends up being undefined.

On one hand, this issue does not appear to be related to React Native. On the other, it's kind of odd because the HTTP client factory does now work like this. 🤔
Am I missing something?

I'll happily open a PR to address this.

[welcome]

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment.
Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

• "Priority" labels will show how urgent this is for the team.
• "Status" labels will show if this is ready to be worked on, blocked, or in progress.
• "Need" labels will indicate if additional input or analysis is required.

Finally, remember to use https://discuss.ipfs.io if you just need general support.

[achingbrain]

Interesting, I think this is a quirk of React Native. In node and the browser this doesn't fail:

const u1 = new URL('http://example.org')
const u2 = new URL(u1)

But it looks like it does in RN. MDN says the argument to the URL constructor should be a string so I guess RN is technically correct here but other environments are more lax.

Anyway #3351 should fix this, thanks for bringing this up.

[acostalima]

Interesting, I think this is a quirk of React Native. In node and the browser this doesn't fail:

const u1 = new URL('http://example.org')
const u2 = new URL(u1)

But it looks like it does in RN. MDN says the argument to the URL constructor should be a string so I guess RN is technically correct here but other environments are more lax.

Anyway #3351 should fix this, thanks for bringing this up.

@achingbrain I missed that, sorry! Thanks for pointing it out. 💪