Make 1.2.1 compatible with node v0.10.x;

[lattmann]

Caveats

• two https tests are failing on windows (node v0.10.36 + Windows, probably related to #17 )
• had to increase timeout for one test from 510ms to 550ms
• tried to minimize all changes
• tried to 'merge' 0.2.x node v0.10.x specific code into 1.2.1 (master branch)

  Tests

• all http tests and one https test ran with node v0.10.36 + Windows (had to set one environment variable NODE_TLS_REJECT_UNAUTHORIZED="0"
• all tests ran with node v0.11.16 + OS X

We need this library to work with node 0.10.x, 0.11.x, and 0.12.x in our project.

Sorry if i did not format the code correctly.

[fengmk2]

@lattmann Nice job!

[fengmk2]

Should add 0.10.x back to travis-ci

[lattmann]

@fengmk2 Thanks for your feedback. Let me know if I have to change anything to get this pull request accepted. (Have not found a CONTRIBUTING.md in the repo.)

I am sorry about the coverage results... Mainly the if (node10) branches are not covered. If you have suggestion how to improve the coverage, i can make some changes. My ideas:

• use the debug library it is available under node v10-v12, then we can remove the node10 conditional from the top of the scripts.
• put the node v10 https specific functions into a separate file, and include it with the if (node10) conditional, maybe the coverage report will not include the unused file/module under node v11 and v12.

We will depend on the fork, until this library supports node v0.10-v0.12.

[lattmann]

I am not sure that I can fix the two failing https tests.

I looked at closely why 0.2.4 tag and 0.2.x branch are green.

Here is what we have found:

• https://travis-ci.org/node-modules/agentkeepalive/builds/55468842 - runs a single test because of this line: https://github.com/node-modules/agentkeepalive/blob/0.2.x/test/agent.test.js#L433
• removing the only from this branch (or release) we get 3 failing tests:
  • 1 http test, where i already increased the timeout, mentioned above
  • 2 https tests (under node v0.10): https://travis-ci.org/node-modules/agentkeepalive/jobs/56717439 this might be a test for #18, although i am not sure that it is the same issue, but could be.

Code coverage got significantly increased by adding node v0.10 to travis.

I am not planning to make any other changes, unless you ask for it.

[fengmk2]

@lattmann Thanks again, I need to double check the 0.10 agent codebase. It's need some more time.

[lattmann]

@fengmk2 sounds good, we understand. Thanks. We will use our fork in the meantime.

[fengmk2]

socket.destroyed on 0.10.x always be undefined....

[fengmk2]

everything seem fine, I will fix some style and merge this.

:+1:

[lattmann]

Thanks.

[fengmk2]

release 2.0.0

[lattmann]

Thanks for the fast response and merge.

A few side notes (I am not suggesting anything ...):

Looking into the node source code... and running the tests using a debugger. Sometimes the net.createConnection function returns with a socket sometimes with a CleartextStream object.

The CleartextStream object has a socket property, which has the destroyed property.

Probably we can check this when we create the connection and just put the s.socket in the this.sockets object.

[fengmk2]

I debug again, s.socket.destroyed still false...

CLIENT socket onClose
s.destroyed: undefined, s.socket.destroyed: false

[lattmann]

Your change fixed the tests, I just left this comment here for future reference. :)