options.uri is a required argument when using default uri

[neverfox]

I make heavy use of the defaults method to to create a request object with a fixed URI and then setting the other options dynamically. As of 2.54.0, I'm now getting options.uri is a required argument when trying to make a request on that object (where I've already set the uri). Downgrading to 2.53.0 clears it up. I didn't see anything in the changelog that would indicate that this would break.

[simov]

It was introduced here #1469 @froatsnook can you take a look at it? :)

[froatsnook]

Oops! I'm on it.

[froatsnook]

@neverfox I can't reproduce this.

$ npm show request | grep 'version:'
  version: '2.54.0',
$ node request-test-defaults.js
OK
OK

Could you please run the test program below and let me know whether prints OK or FAIL? If it prints OK, could you modify it to reproduce your error? Or provide some other example that breaks?

'use strict'

var http = require('http')
  , request = require('request')
  , url = require('url')

var s = http.createServer(function(req, res) {
  res.statusCode = 200
  res.writeHead(200, { 'Content-Type': 'application/json' })
  res.end(JSON.stringify(req.headers))
})

s.listen(6767)

var r = request.defaults({
  uri: 'http://localhost:6767/test'
})

var count = 0

r({
  json: true,
  headers: {
    'X-TEST': 'test 1'
  }
}, function(err, res, body) {
  if (err || body['x-test'] !== 'test 1') {
    console.log('FAIL')
  } else {
    console.log('OK')
  }

  if (++count === 2) {
    s.close()
  }
})

r({
  json: true,
  headers: {
    'X-TEST': 'test 2'
  }
}, function(err, res, body) {
  if (err || body['x-test'] !== 'test 2') {
    console.log('FAIL')
  } else {
    console.log('OK')
  }

  if (++count === 2) {
    s.close()
  }
})

[neverfox]

Yes, I will. Thanks for the code.

[simov]

Actually @neverfox your bug report is a bit vague:

I make heavy use of the defaults method to to create a request object with a fixed URI and then setting the other options dynamically.

Can you give us a short code example of what are you using, and what you think might cause the issue?

[neverfox]

Here is the test code above, modified with some code from my project, that recreates the error:

'use strict'

var Promise = require('bluebird')
  , http = require('http')
  , request = require('request')
  , url = require('url')

var s = http.createServer(function(req, res) {
  res.statusCode = 200
  res.writeHead(200, { 'Content-Type': 'application/json' })
  res.end(JSON.stringify(req.headers))
})

s.listen(6767)

function Pool() {
  var href = url.format({
    protocol: 'http:',
    hostname: 'localhost',
    pathname: 'test',
    port: 6767
  });

  this.request = Promise.promisifyAll(
    request.defaults({
      uri: href
    })
  );
}

var pool = new Pool();

var count = 0

pool.request.get({}, function(err, res, body) {
  if (err) {
    console.log('FAIL', err.message)
  } else {
    console.log('OK')
  }

  if (++count === 2) {
    s.close()
  }
})

pool.request.get({
  qs: {
    cmd: 'plan'
  }
}, function(err, res, body) {
  if (err) {
    console.log('FAIL', err.message)
  } else {
    console.log('OK')
  }

  if (++count === 2) {
    s.close()
  }
})

2.53.0:

OK
OK

2.54.0:

FAIL options.uri is a required argument
FAIL options.uri is a required argument

2.54.0 (without get verb):

OK
OK

The promisification is there to be sure that I'm recreating the code as it exists, but it doesn't make a difference if I take it out.

[neverfox]

The cause seems to be that it doesn't work if I use a verb (.get) rather than the request directly. @froatsnook's original code fails with r.get.

[simov]

It turned out that this bug is not related to baseUrl, but instead #1430

Fixed here #1518 @neverfox can you give it a try?