Cannot change prefixUrl: The API forces a hostname when it is unknown and with no way to override it later

rooftopsparrow commented 3 years ago

Describe the bug

Node.js version: v14.15.4
OS & version: macOS Catalina 10.15.7

TL;DR: When the host is not known, there is no way to create a client for a specific API and fill it in later with the .extend api, or with any API that I can see in the docs.

Assume that I use a discovery mechanism for resolving the current hosts available for a service. This "service pool" has ip and port information clients can to connect to; there is no dns name or load balancer involved. This discovery mechanism is asynchronous as hosts go in and out of service, so the lookup mechanic is a Promise.

This means the client does not know the host it will connect to up front, but a client should be able to be created because it knows the path it will be talking to and behavior of each endpoint. It is desirable to encode the behavior of the client up front and inject the host name it is talking to "just in time".

In the docs, it claims that I can change the prefixUrl but in reality this doesn't seem to be true, or I have yet to see how this can work when the domain is different from what was previously provided. In the docs it shows an example of changing the domain name in a beforeRequest hook, just as I have tried below in the script, yet does not seem to actually work.

I seem to be stuck with no way forward using hooks or any api provided. It appears that the strictness of the prefixUrl checking does not allow adjusting the endpoint host dynamically.

I am submitting this is a bug and not a feature request because the docs seem to be describing the exact feature I want but also seems to not work as I would expect. However, please feel free to reclassify it as needed.

Finally, thanks for this lib and your time! ❤️

Actual behavior

no slash TypeError [ERR_INVALID_URL]: Invalid URL: foobar
    at onParseError (internal/url.js:257:9)
    at new URL (internal/url.js:333:5)
    at Object.exports.default (~/got-testing/node_modules/got/dist/source/core/utils/options-to-url.js:35:17)
    at normalizeArguments (~/got-testing/node_modules/got/dist/source/core/index.js:480:51)
    at got (~/got-testing/node_modules/got/dist/source/create.js:112:39)
    at Function.got.<computed> [as get] (~/got-testing/node_modules/got/dist/source/create.js:224:42)
    at ~/got-testing/got-testing.js:36:27
    at processTicksAndRejections (internal/process/task_queues.js:97:5) {
  input: 'foobar',
  code: 'ERR_INVALID_URL'
}
-----
with slash Error: `input` must not start with a slash when using `prefixUrl`
    at normalizeArguments (~/got-testing/node_modules/got/dist/source/core/index.js:478:23)
    at got (~/got-testing/node_modules/got/dist/source/create.js:112:39)
    at Function.got.<computed> [as get] (~/got-testing/node_modules/got/dist/source/create.js:224:42)
    at ~/got-testing/got-testing.js:42:27
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
-----
dummy prefix RequestError: Cannot change `prefixUrl` from http://example.com/ to http://127.0.0.1:3000: http://example.com/foobar
    at Request._destroy (~/got-testing/node_modules/got/dist/source/core/index.js:1369:21)
    at Request.destroy (internal/streams/destroy.js:38:8)
    at ~/got-testing/node_modules/got/dist/source/core/index.js:348:26
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at Object.set [as prefixUrl] (~/got-testing/node_modules/got/dist/source/core/index.js:495:31)
    at Got.extend.hooks.beforeRequest (~/got-testing/got-testing.js:25:27)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async Request._makeRequest (~/got-testing/node_modules/got/dist/source/core/index.js:1034:28)
    at async ~/got-testing/node_modules/got/dist/source/core/index.js:328:17 {
  code: undefined,
  timings: undefined
}
-----
got data null

Expected behavior

got data {foo: 'bar'}

Code to reproduce

const Got = require('got')
const http = require('http')

// Mock server, assumption this is hosted somewhere in the network with some internal ip, not locally
const port = 3000
function listen () {
  return new Promise((resolve, reject) => {
    const server = http.createServer((_, res) => {
      res.setHeader('content-type', 'application/json').end(JSON.stringify({ foo: 'bar' }))
    })
    server.listen(3000, (err) => err ? reject(err) : resolve(server))
  })
}

// Mock discovery component
const serviceDiscovery = {
  getProvider () {
    return Promise.resolve({ host: '127.0.0.1', port })
  }
}

const client = Got.extend({
  hooks: {
    beforeRequest: [
      async options => {
        const { host, port } = await serviceDiscovery.getProvider('foobar')
        options.prefixUrl = `http://${host}:${port}`
      }
    ]
  }
})

;(async () => {
  var server = await listen()
  try {
    var data = null
    try {
      data = await client.get('foobar').json()
    } catch (err) {
      console.error('no slash', err)
      console.error('-----')
    }
    try {
      data = await client.get('/foobar').json()
    } catch (err) {
      console.error('with slash', err)
      console.error('-----')
    }
    try {
      data = await client.extend({ prefixUrl: 'http://example.com' }).get('foobar')
    } catch (err) {
      console.error('dummy prefix', err)
      console.error('-----')
    }
  } finally {
    server.close()
  }
  console.log('got data', data)
})()

Checklist

[x] I have read the documentation.
[x] I have tried my code with the latest version of Node.js and Got.

szmarczak commented 3 years ago

https://github.com/sindresorhus/got#prefixurl

Tip: You can change prefixUrl using hooks as long as the URL still includes the prefixUrl. If the URL doesn't include it anymore, it will throw.

IOW, you want to change the entire URL, not the prefixUrl itself.

rooftopsparrow commented 3 years ago

Thank you for taking a look, I was not aware that you could change the url property directly.

I just want to share the snippet of the the code below: changing the URL does work, however it still requires a dummy prefixUrl.

const client = Got.extend({
  prefixUrl: 'http://placeholder',
  hooks: {
    beforeRequest: [
      async options => {
        const { host, port } = await serviceDiscovery.getProvider('foobar')
        options.url.host = host
        options.url.port = port
      }
    ]
  }
})

sindresorhus / got