honor httpAgent/httpsAgent when setting ntlm flag (take 2)

[smokhov]

• Closes honor httpAgent/httpsAgent when setting ntlm flag #1209

This PR supersedes PR #1209 (by @unilynx) by adding tests for 'ntlm' to be together with httpAgent and httpsAgent. I used two different approaches of creating the tests agents inspired from other tests where they were used and by making sure ntlm was set to true. I've also sync'd it with master.

[w666]

I tried to remove this locally, rebuild the project and run new tests, but for some reason tests still pass.
I would assume it should fail without it.
Do I understand it right? Could you please double check it?

[smokhov]

I'll have a look when I get a chance hopefully soon. It is entirely possible there is a bug in the tests :)

[smokhov]

@w666 -- so I've done some testing here. TL;DR, the proposes change overall makes sense. To test it i had to dig a bit. Now the question to you would be how would make this available from scope of tests. Once solved, then can fix the tests to query for the agents.

I've pushed the instrumentation commit a54e32d. Most of some of it to be reverted once solution is found. I've doubled the number of request-response tests with and without change based on the options, and they all pass, but if I console-log the http(s)Agent at the right spot, they are

Summary:

• https.ts instantiates either AxiosInstance or NtlmClient stored in req depending on the options ntlm or not.

  node-soap/src/http.ts

  Line 182 in 79a0363

  if (exoptions !== undefined && exoptions.ntlm) {

• the OG proposed change, { httpAgent: exoptions.httpAgent, httpsAgent: exoptions.httpsAgent } passes this as a config parameter to NtlmClient at the constructor (axios-ntlm config). I actually tested and think a better way to pass all of the options, (I kept them commented out):

      const ntlmReq = NtlmClient(
        {
          username: exoptions.username,
          password: exoptions.password,
          workstation: exoptions.workstation || '',
          domain: exoptions.domain || '',
        },
        // Change wanted in the OG PR:
        //{ httpAgent: exoptions.httpAgent, httpsAgent: exoptions.httpsAgent },
        // A better change per axios-ntlm API?
        //options,
      );

• then actually the next line has req = ntlmReq(options); so supposedly the agents would have been passed, but this call appears to be a Promise call and the agents information seems not to be passed further properly. Doing it explicitly and eagerly at the constructor appears to change the agent properly from my logs
• to test for this properly we need to examine the resulting req object from the request() call

  node-soap/src/http.ts

  Line 179 in 79a0363

  public request(rurl: string, data: any, callback: (error: any, res?: any, body?: any) => any, exheaders?: IHeaders, exoptions?: IExOptions) {

• currently the return value from that call is inhibited through HttpClient._invoke()in _defineMethod():

  node-soap/src/client.ts

  Line 263 in 79a0363

  this._invoke(

  node-soap/src/client.ts

  Line 576 in 79a0363

  return this.httpClient.request(

If we can get access to that return value of reqat the scope of the tests (I could not find an easy way to do so), then we can compare the http(s)Agent values from the actual to the requested and fail if they don't match by commenting out the change. I am currently ouf of energy to dig deeper, so if you have insights and suggestions, please feel free. Otherwise, the change makes sense as I said.