Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Add support for sending events through proxy #397

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Add support for sending events through proxy
For both HTTP and HTTPS endpoints. Uses tunnel-agent for https.
  • Loading branch information
ndmanvar committed Nov 17, 2017
commit 6cce30b3eb27be9bfdb6a3c9ee814d4613e2b227
64 changes: 57 additions & 7 deletions lib/transports.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ var timeoutReq = require('timed-out');

var http = require('http');
var https = require('https');
var tunnel = require('tunnel-agent');

var agentOptions = {keepAlive: true, maxSockets: 100};
var agentOptions = {
keepAlive: true,
maxSockets: 100
};
var httpAgent = new http.Agent(agentOptions);
var httpsAgent = new https.Agent(agentOptions);

Expand All @@ -21,7 +25,7 @@ function HTTPTransport(options) {
this.agent = httpAgent;
}
util.inherits(HTTPTransport, Transport);
HTTPTransport.prototype.send = function(client, message, headers, eventId, cb) {
HTTPTransport.prototype.send = function (client, message, headers, eventId, cb) {
var options = {
hostname: client.dsn.host,
path: client.dsn.path + 'api/' + client.dsn.project_id + '/store/',
Expand All @@ -31,22 +35,39 @@ HTTPTransport.prototype.send = function(client, message, headers, eventId, cb) {
ca: client.ca,
agent: this.agent
};

// set path apprpriately when using http endpoint + proxy, set proxy headers appropriately when using https endpoint + proxy
if (this.options.hasOwnProperty('proxyHost')) {
if (client.dsn.protocol === 'http') {
options.path = 'https://' + client.dsn.host + ':' + client.dsn.port + client.dsn.path + 'api/' + client.dsn.project_id + '/store/';
delete options.hostname; // only 'host' should be set when using proxy
Copy link
Contributor

Choose a reason for hiding this comment

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

slightly better comment here for what will happen if you send hostname or a link to a resource that justifies why?

} else {
this.options.agent.proxyOptions.headers = {
'Content-Type': 'application/octet-stream',
host: client.dsn.host + ':' + client.dsn.port
}
}
}

for (var key in this.options) {
if (this.options.hasOwnProperty(key)) {
options[key] = this.options[key];
}
}

// prevent off heap memory explosion
var _name = this.agent.getName({host: client.dsn.host, port: client.dsn.port});
var _name = this.agent.getName({
host: client.dsn.host,
port: client.dsn.port
});
var _requests = this.agent.requests[_name];
if (_requests && Object.keys(_requests).length > client.maxReqQueueCount) {
// other feedback strategy
client.emit('error', new Error('client req queue is full..'));
return;
}

var req = this.transport.request(options, function(res) {
var req = this.transport.request(options, function (res) {
res.setEncoding('utf8');
if (res.statusCode >= 200 && res.statusCode < 300) {
client.emit('logged', eventId);
Expand All @@ -63,17 +84,16 @@ HTTPTransport.prototype.send = function(client, message, headers, eventId, cb) {
client.emit('error', e);
cb && cb(e);
}

// force the socket to drain
var noop = function() {};
var noop = function () {};
res.on('data', noop);
res.on('end', noop);
});

timeoutReq(req, client.sendTimeout * 1000);

var cbFired = false;
req.on('error', function(e) {
req.on('error', function (e) {
client.emit('error', e);
if (!cbFired) {
cb && cb(e);
Expand All @@ -89,10 +109,40 @@ function HTTPSTransport(options) {
this.options = options || {};
this.agent = httpsAgent;
}

function HTTPProxyTransport(options) {
this.defaultPort = 80;
this.transport = http;
this.options = options || {};
this.agent = httpAgent;
this.options.host = options.proxyHost;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about just using host port etc. instead? If we pass options object to this constructor, it's by definition mentioned to be used by proxy, therefore using a prefix proxyHost seems redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or it may be useful to distinguish http from httpProxy options. It's up to you really.

this.options.port = options.proxyPort;
}

function HTTPSProxyTransport(options) {
this.transport = https;
this.options = options || {};
this.agent = httpsAgent;
this.options.agent = tunnel.httpsOverHttp({
proxy: {
host: options.proxyHost,
port: options.proxyPort,
proxyAuth: null // TODO: Add ability to specify creds/auth
Copy link
Author

Choose a reason for hiding this comment

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

will take care of this (for HTTPProxyTransport also)

},
keepAlive: agentOptions.keepAlive,
maxSockets: agentOptions.maxSockets
});
}

util.inherits(HTTPSTransport, HTTPTransport);
util.inherits(HTTPProxyTransport, HTTPTransport);
util.inherits(HTTPSProxyTransport, HTTPTransport);

module.exports.http = new HTTPTransport();
module.exports.https = new HTTPSTransport();
module.exports.Transport = Transport;
module.exports.HTTPTransport = HTTPTransport;
module.exports.HTTPSTransport = HTTPSTransport;

module.exports.HTTPProxyTransport = HTTPProxyTransport;
module.exports.HTTPSProxyTransport = HTTPSProxyTransport;
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"lsmod": "1.0.0",
"stack-trace": "0.0.9",
"timed-out": "4.0.1",
"tunnel-agent": "^0.6.0",
"uuid": "3.0.0"
},
"devDependencies": {
Expand Down