Skip to content

fix: handle idle socket error #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
sudo: false
language: node_js
node_js:
- '0.10'
- '0.11'
- '0.12'
- 'iojs-1'
script: "npm run test-travis"
after_script: "npm install coveralls@2 && cat ./coverage/lcov.info | coveralls"
- '4.3.2'
- '4'
- '5.3.0'
- '5'
script: 'npm run ci'
after_script: 'npm run codecov'
15 changes: 9 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

[![NPM version][npm-image]][npm-url]
[![build status][travis-image]][travis-url]
[![Test coverage][coveralls-image]][coveralls-url]
[![Gittip][gittip-image]][gittip-url]
[![Circle status][circle-image]][circle-url]
[![Appveyor status][appveyor-image]][appveyor-url]
[![Test coverage][codecov-image]][codecov-url]
[![David deps][david-image]][david-url]
[![node version][node-image]][node-url]
[![npm download][download-image]][download-url]
Expand All @@ -12,10 +13,12 @@
[npm-url]: https://npmjs.org/package/agentkeepalive
[travis-image]: https://img.shields.io/travis/node-modules/agentkeepalive.svg?style=flat
[travis-url]: https://travis-ci.org/node-modules/agentkeepalive
[coveralls-image]: https://img.shields.io/coveralls/node-modules/agentkeepalive.svg?style=flat
[coveralls-url]: https://coveralls.io/r/node-modules/agentkeepalive?branch=master
[gittip-image]: https://img.shields.io/gittip/fengmk2.svg?style=flat
[gittip-url]: https://www.gittip.com/fengmk2/
[circle-image]: https://circleci.com/gh/node-modules/agentkeepalive.svg?style=svg
[circle-url]: https://circleci.com/gh/node-modules/agentkeepalive
[appveyor-image]: https://ci.appveyor.com/api/projects/status/k7ct4s47di6m5uy2?svg=true
[appveyor-url]: https://ci.appveyor.com/project/fengmk2/agentkeepalive
[codecov-image]: https://codecov.io/github/node-modules/agentkeepalive/coverage.svg?branch=master
[codecov-url]: https://codecov.io/github/node-modules/agentkeepalive?branch=master
[david-image]: https://img.shields.io/david/node-modules/agentkeepalive.svg?style=flat
[david-url]: https://david-dm.org/node-modules/agentkeepalive
[node-image]: https://img.shields.io/badge/node.js-%3E=_0.11-green.svg?style=flat-square
Expand Down
28 changes: 28 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
environment:
global:
npm_registry: https://registry.npmjs.com

matrix:
- nodejs_version: "0.10"
- nodejs_version: "0.12"
- nodejs_version: "4.3.2"
- nodejs_version: "4"
- nodejs_version: "5.3.0"
- nodejs_version: "5"

# Install scripts. (runs after repo cloning)
install:
# Get the latest stable version of Node.js or io.js
- ps: Install-Product node $env:nodejs_version
# install modules
- npm i

# Post-install test scripts.
test_script:
# Output useful info for debugging.
- node --version
- npm --version
- npm run ci

# Don't actually build.
build: off
19 changes: 19 additions & 0 deletions circle.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
machine:
node:
version: '4'

dependencies:
override:
- nvm ls

test:
override:
- nvm i 0.10 && rm -rf node_modules && npm i && npm run ci && npm run codecov
- nvm i 0.12 && rm -rf node_modules && npm i && npm run ci && npm run codecov
- nvm i 4.3.2 && rm -rf node_modules && npm i && npm run ci && npm run codecov
- nvm i 4 && rm -rf node_modules && npm i && npm run ci && npm run codecov
- nvm i 5.3.0 && rm -rf node_modules && npm i && npm run ci && npm run codecov
- nvm i 5 && rm -rf node_modules && npm i && npm run ci && npm run codecov

# https://circleci.com/docs/language-nodejs
# https://discuss.circleci.com/t/testing-multiple-versions-of-node/542
28 changes: 18 additions & 10 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

// copy from https://github.com/joyent/node/blob/master/lib/_http_agent.js
// copy from https://github.com/nodejs/node/blob/v4.x/lib/_http_agent.js

'use strict';

Expand Down Expand Up @@ -71,6 +71,7 @@ function Agent(options) {
self.keepAliveMsecs = self.options.keepAliveMsecs || 1000;
self.keepAlive = self.options.keepAlive || false;
// free keep-alive socket timeout. By default free socket do not have a timeout.
// keepAliveTimeout should be rename to `freeSocketKeepAliveTimeout`
self.keepAliveTimeout = self.options.keepAliveTimeout || 0;
// working socket timeout. By default working socket do not have a timeout.
self.timeout = self.options.timeout || 0;
Expand Down Expand Up @@ -116,6 +117,13 @@ function Agent(options) {
self.removeSocket(socket, options);
freeSockets.push(socket);

// Add a default error handler to avoid Unhandled 'error' event throw on idle socket
// https://github.com/node-modules/agentkeepalive/issues/25
// https://github.com/nodejs/node/pull/4482 (fixed in >= 4.4.0 and >= 5.4.0)
if (socket.listeners('error').length === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dead-horse fixed

socket.once('error', freeSocketErrorListener);
}

// set free keepalive timer
socket.setTimeout(self.keepAliveTimeout);
}
Expand All @@ -130,6 +138,13 @@ function Agent(options) {
util.inherits(Agent, EventEmitter);
exports.Agent = Agent;

function freeSocketErrorListener(err) {
var socket = this;
debug('SOCKET ERROR on FREE socket:', err.message, err.stack);
socket.destroy();
socket.emit('agentRemove');
}

Agent.defaultMaxSockets = Infinity;

Agent.prototype.createConnection = net.createConnection;
Expand Down Expand Up @@ -176,6 +191,8 @@ Agent.prototype.addRequest = function(req, options) {
var socket = this.freeSockets[name].shift();
debug('have free socket');

socket.removeListener('error', freeSocketErrorListener);

// restart the default timer
socket.setTimeout(this.timeout);

Expand Down Expand Up @@ -257,14 +274,6 @@ Agent.prototype.createSocket = function(req, options) {
// set the default timer
s.setTimeout(self.timeout);

// Add a default error handler to avoid Unhandled 'error' event throw
// https://github.com/node-modules/agentkeepalive/issues/25
function onError(err) {
debug('CLIENT socket onError: %s', err);
self.emit('error', err);
}
s.on('error', onError);

function onRemove() {
// We need this function for cases like HTTP 'upgrade'
// (defined by WebSockets) where we need to remove a socket from the
Expand All @@ -274,7 +283,6 @@ Agent.prototype.createSocket = function(req, options) {
s.removeListener('close', onClose);
s.removeListener('free', onFree);
s.removeListener('agentRemove', onRemove);
s.removeListener('error', onError);
// remove timer
s.setTimeout(0, onTimeout);
}
Expand Down
4 changes: 1 addition & 3 deletions lib/agent.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
/**!
* agentkeepalive - lib/agent.js
*
/**
* refer:
* * @atimb "Real keep-alive HTTP agent": https://gist.github.com/2963672
* * https://github.com/joyent/node/blob/master/lib/http.js
Expand Down
34 changes: 19 additions & 15 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@
"description": "Missing keepalive http.Agent",
"main": "index.js",
"files": [
"index.js", "lib/"
"index.js",
"lib"
],
"scripts": {
"test": "mocha -R spec -t 5000 -r should-http test/*.test.js",
"test-cov": "node node_modules/.bin/istanbul cover node_modules/.bin/_mocha -- -t 5000 -r should-http test/*.test.js",
"test-travis": "node node_modules/.bin/istanbul cover node_modules/.bin/_mocha --report lcovonly -- -t 5000 -r should-http test/*.test.js",
"jshint": "jshint .",
"autod": "autod -w --prefix '~' && npm run cnpm",
"cnpm": "npm install --registry=https://registry.npm.taobao.org",
"contributors": "contributors -f plain -o AUTHORS"
"test-cov": "istanbul cover node_modules/mocha/bin/_mocha -- -t 5000 -r should-http test/*.test.js",
"ci": "npm run lint && npm run test-cov",
"lint": "jshint .",
"autod": "autod -w --prefix '~'",
"contributors": "contributors -f plain -o AUTHORS",
"codecov": "npm i codecov && codecov"
},
"repository": {
"type": "git",
Expand All @@ -24,22 +25,25 @@
},
"keywords": [
"http",
"https",
"agent",
"keepalive"
"keepalive",
"agentkeepalive"
],
"dependencies": {

},
"dependencies": {},
"devDependencies": {
"autod": "*",
"contributors": "*",
"istanbul": "*",
"mocha": "*",
"pedding": "~1.0.0",
"should": "~4.0.4",
"should-http": "~0.0.2"
"pedding": "1",
"should": "4",
"should-http": "~0.0.2",
"jshint": "^2.9.1"
},
"engines": {
"node": ">= 0.10.0"
},
"engines": { "node": ">= 0.10.0" },
"author": "fengmk2 <[email protected]> (http://fengmk2.com)",
"license": "MIT"
}
46 changes: 42 additions & 4 deletions test/agent.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
/*!
* agentkeepalive - test/agent.test.js
*
/**
* Copyright(c) 2012 - 2013 fengmk2 <[email protected]>
* Copyright(c) node-modules
* MIT Licensed
Expand All @@ -19,7 +17,7 @@ require('should-http');
var pedding = require('pedding');
var Agent = require('../');

describe('agent.test.js', function () {
describe('test/agent.test.js', function () {

var agentkeepalive = new Agent({
keepAliveTimeout: 1000,
Expand Down Expand Up @@ -810,4 +808,44 @@ describe('agent.test.js', function () {
});
});

describe('mock idle socket error', function() {
it('should idle socket emit error event', function(done) {
var agent = new Agent();

var options = {
host: 'www.taobao.com',
port: 80,
path: '/',
agent: agent
};

var socketKey = agent.getName(options);
var req = http.get(options, function(res) {
var size = 0;
res.on('data', function(chunk) {
size += chunk.length;
});
res.on('end', function() {
size.should.above(0);
Object.keys(agent.freeSockets).should.length(0);
process.nextTick(function() {
should.exist(agent.freeSockets[socketKey]);
agent.freeSockets[socketKey].should.length(1);
setTimeout(function() {
// agent should catch idle socket error event
agent.freeSockets[socketKey][0].emit('error', new Error('mock read ECONNRESET'));

setTimeout(function() {
// error socket should be destroy and remove
Object.keys(agent.freeSockets).should.length(0);
done();
}, 10);
}, 10);
});
});
res.resume();
});
req.on('error', done);
});
});
});