Discuss: allow any options to be passed to request

[hypesystem]

I think it seems a bit silly to have the current restriction on what options are passed on to request. I think we should probably just allow all options to be passed through.

[eladnava]

Are you referring to the request call on sender.js#L112?

[hypesystem]

Yes :)

Right now we allow the options proxy, maxSockets, timeout, and strictSSL (new). It would make sense to allow any options to be set, and just ensure that we set the required fields as they must be set.

[eladnava]

@hypesystem Sounds good.

However, I think we need to change things a bit.

Right now, this is how you pass in configuration that influences the request:

var options = {
   proxy: 'http://myproxy.com',
   maxSockets: 100,
   timeout: 1000,
   strictSSL: false
};

// Set up the sender with you API key
var sender = new gcm.Sender('YOUR_API_KEY_HERE', options);

If we want to make it possible to override any request param, I think that we should pass in those params in a nested request object as follows:

var options = {
   request: {
      proxy: 'http://myproxy.com',
      maxSockets: 100,
      timeout: 1000,
      strictSsl: false
   }
};

What do you think?

[hypesystem]

I think that would probably be the best approach at some point. At the moment, though, I think we should value not introducing breaking changes. The only options the Sender can take at the moment are options for request. Maybe we should just change the name we use for the parameter to requestOptions?

Until we feel ready for a major version bump (and that requires figuring out which breaking changes we want to make), this version would be preferable

var requestOptions = {
   proxy: 'http://myproxy.com',
   maxSockets: 100,
   timeout: 1000,
   strictSsl: false
};

// Set up the sender with you API key
var sender = new gcm.Sender('YOUR_API_KEY_HERE', requestOptions);

[eladnava]

@hypesystem Excellent idea.

So, the 2 changes that will be made are:

1. Remove the hard-coded options and simply add the sender options elements to the post_options
2. Modify the documentation, mentioning how to override the request options (specifying requestOptions as the sender() param) and linking to all the available configuration elements in the request GitHub page

There are 2 issues with this:

1. Note that this will introduce a breaking change for the strictSsl parameter which is referred to as strictSSL in request. What should we do in this case?
2. Do we want developers to be able to override our own post_options? headers for example?

[hypesystem]

I think the post_options we always set should have priority. Lodash has a nice function, defaults, which would let us do this to get that effect:

post_options = _.defaults(post_options, requestOptions);

The strictSSL change has not yet been release, so it would not actually be a breaking change :)

[eladnava]

@hypesystem Should we really include Lodash just for this? I was thinking of defaulting the options to the provided ones, manually setting our own (via line-by-line set calls), and providing it to request.

[hypesystem]

There are pros and cons. lodash is a pretty common dependency, but either solution would be fine by me :)

[eladnava]

@hypesystem Went with lodash for its _.defaultsDeep(), which makes it possible for us to allow appending headers to the request =)