Skip to content
This repository was archived by the owner on Aug 20, 2024. It is now read-only.

Add connectTimeout option #26

Merged
merged 1 commit into from
Oct 20, 2020
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
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,24 @@ app.use(proxy('httpbin.org', {
}));
```

#### connectTimeout

By default, node does not express a timeout on connections.
Use connectTimeout option to impose a specific timeout on the inital connection. (`connect` for http requests and `secureConnect` for https)
This is useful if there are dns, network issues, or if you are uncertain if the destination is reachable.
Timed-out requests will respond with 504 status code and a X-Timeout-Reason header.

```js
app.use(proxy('httpbin.org', {
connectTimeout: 2000 // in milliseconds, two seconds
}));
```


#### timeout

By default, node does not express a timeout on connections.
Use timeout option to impose a specific timeout.
Use timeout option to impose a specific timeout. This includes the time taken to make the connection and can be used with or without `connectTimeout`.
Timed-out requests will respond with 504 status code and a X-Timeout-Reason header.

```js
Expand Down
30 changes: 26 additions & 4 deletions app/steps/sendProxyRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,42 @@ function sendProxyRequest(Container) {
});
rsp.on('error', reject);
});
var abortRequest = function() {
proxyReq.abort();
};
var timeoutDuration = null;

proxyReq.on('socket', function(socket) {
if (options.timeout) {
socket.setTimeout(options.timeout, function() {
proxyReq.abort();
var isSecure = Container.proxy.isSecure;
var timeout = options.timeout;
var connectTimeout = options.connectTimeout;
// "secureConnect" includes time taken for "lookup" (dns), "connect" and "ready" events, as well as tls handshake.
var eventListener = isSecure ? 'secureConnect' : 'connect';

if (connectTimeout) {
timeoutDuration = connectTimeout;
socket.setTimeout(connectTimeout, abortRequest);

socket.on(eventListener, function() {
if (timeout) {
timeoutDuration = timeout;
socket.setTimeout(timeout, abortRequest);
} else {
// 0 to reset to the default of no timeout for the rest of the request
socket.setTimeout(0);
}
});
} else if (timeout) {
timeoutDuration = timeout;
socket.setTimeout(timeout, abortRequest);
}
});

// TODO: do reject here and handle this later on
proxyReq.on('error', function(err) {
// reject(error);
if (err.code === 'ECONNRESET') {
ctx.set('X-Timout-Reason', 'koa-better-http-proxy timed out your request after ' + options.timeout + 'ms.');
ctx.set('X-Timout-Reason', 'koa-better-http-proxy timed out your request after ' + timeoutDuration + 'ms.');
Copy link
Contributor Author

@jgodson jgodson Oct 15, 2020

Choose a reason for hiding this comment

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

There's a typo in this header, but that could be a breaking change technically so I left it alone

ctx.set('Content-Type', 'text/plain');
ctx.status = 504;
resolve(Container);
Expand Down
1 change: 1 addition & 0 deletions lib/requestOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ function parseHost(Container) {
return {
host: parsed.hostname,
port: parsed.port || (ishttps ? 443 : 80),
isSecure: ishttps,
module: ishttps ? https : http,
};
}
Expand Down
1 change: 1 addition & 0 deletions lib/resolveOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function resolveOptions(options) {
https: options.https,
port: options.port,
reqAsBuffer: options.reqAsBuffer,
connectTimeout: options.connectTimeout,
timeout: options.timeout,
limit: options.limit,
};
Expand Down
97 changes: 97 additions & 0 deletions test/connectTimeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
var Koa = require('koa');
var agent = require('supertest').agent;
var proxy = require('../');
var proxyTarget = require('./support/proxyTarget');

describe('honors connectTimeout option', function() {
'use strict';

var other, http;
beforeEach(function() {
http = new Koa();
other = proxyTarget(8080, 1000, [{
method: 'get',
path: '/',
fn: function(_, res) { res.sendStatus(200); }
}]);
});

afterEach(function() {
other.close();
});

function assertSuccess(server, done) {
agent(server.callback())
.get('/')
.expect(200)
.end(function(err) {
if (err) {
return done(err);
}
done();
});
}

function assertConnectionTimeout(server, time, done) {
agent(server.callback())
.get('/')
.expect(504)
.expect('X-Timout-Reason', 'koa-better-http-proxy timed out your request after ' + time + 'ms.')
.end(function(err) {
if (err) {
return done(err);
}
done();
});
}

describe('when connectTimeout option is set lower than server connect time', function() {
it('should fail with CONNECTION TIMEOUT', function(done) {
http.use(proxy('http://127.0.0.0', {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to find a seemingly more "legit" way to make this happen, but I couldn't and this seems to work 🤷‍♂️

connectTimeout: 50,
}));

assertConnectionTimeout(http, 50, done);
});
});

describe('when connectTimeout option is set higher than server connect time', function() {
it('should succeed', function(done) {
http.use(proxy('http://localhost:8080', {
connectTimeout: 50,
}));

assertSuccess(http, done);
});
});

describe('when timeout option is also used', function() {
it('should fail with CONNECTION TIMEOUT when timeout is set lower than server response time', function(done) {
http.use(proxy('http://localhost:8080', {
connectTimeout: 100,
timeout: 300,
}));

assertConnectionTimeout(http, 300, done);
});

it('should fail with CONNECTION TIMEOUT based on connectTimeout when a connection cannot be made', function(done) {
http.use(proxy('http://127.0.0.0', {
connectTimeout: 100,
timeout: 300,
}));

assertConnectionTimeout(http, 100, done);
});

it('should succeed when timeout is higher than server response time', function(done) {
http.use(proxy('http://localhost:8080', {
connectTimeout: 100,
timeout: 1200,
}));

assertSuccess(http, done);
});
});

});
1 change: 1 addition & 0 deletions types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ declare namespace koaHttpProxy {
preserveReqSession?: boolean,
reqAsBuffer?: boolean,
reqBodyEncoding?: string | null,
connectTimeout?: number,
timeout?: number,
filter?(ctx: koa.Context): boolean,
proxyReqBodyDecorator?(bodyContent: string, ctx: koa.Context): string | Promise<string>,
Expand Down