From 1e77fc65383061812c0ed53a469185105aad1716 Mon Sep 17 00:00:00 2001 From: Matteo Zambon Date: Thu, 23 Sep 2021 16:52:35 +0100 Subject: [PATCH 1/5] Added logger to Transport and OAuth2 Added a TODO for where to proceed next co-authored-by: beldougie --- lib/connection.js | 9 +++++---- lib/oauth2.js | 11 +++++++---- lib/transport.js | 20 ++++++++++++++------ 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 97ed0b320..d071f9f3f 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -66,6 +66,7 @@ var Connection = module.exports = function(options) { proxyUrl: options.proxyUrl, httpProxy: options.httpProxy }; + oauth2._logger = this._logger; /** * OAuth2 object @@ -79,11 +80,11 @@ var Connection = module.exports = function(options) { /** @private */ if (options.proxyUrl) { - this._transport = new Transport.ProxyTransport(options.proxyUrl); + this._transport = new Transport.ProxyTransport(options.proxyUrl, this._logger); } else if (options.httpProxy) { - this._transport = new Transport.HttpProxyTransport(options.httpProxy); + this._transport = new Transport.HttpProxyTransport(options.httpProxy, this._logger); } else { - this._transport = new Transport(); + this._transport = new Transport(this._logger); } this.callOptions = options.callOptions; @@ -204,7 +205,7 @@ Connection.prototype.initialize = function(options) { if (this.signedRequest) { this.accessToken = this.signedRequest.client.oauthToken; if (Transport.CanvasTransport.supported) { - this._transport = new Transport.CanvasTransport(this.signedRequest); + this._transport = new Transport.CanvasTransport(this.signedRequest, this._logger); } } diff --git a/lib/oauth2.js b/lib/oauth2.js index 5bdaef3cd..6383d1e3e 100644 --- a/lib/oauth2.js +++ b/lib/oauth2.js @@ -7,7 +7,8 @@ var querystring = require('querystring'), _ = require('lodash/core'), - Transport = require('./transport'); + Transport = require('./transport'), + Logger = require('./logger'); var defaults = { loginUrl : "https://login.salesforce.com" @@ -27,6 +28,8 @@ var defaults = { * @param {String} options.redirectUri - URI to be callbacked from Salesforce OAuth2 authorization service. */ var OAuth2 = module.exports = function(options) { + this._logger = options._logger || new Logger(); + if (options.authzServiceUrl && options.tokenServiceUrl) { this.loginUrl = options.authzServiceUrl.split('/').slice(0, 3).join('/'); this.authzServiceUrl = options.authzServiceUrl; @@ -42,11 +45,11 @@ var OAuth2 = module.exports = function(options) { this.clientSecret = options.clientSecret; this.redirectUri = options.redirectUri; if (options.proxyUrl) { - this._transport = new Transport.ProxyTransport(options.proxyUrl); + this._transport = new Transport.ProxyTransport(options.proxyUrl, this._logger); } else if (options.httpProxy) { - this._transport = new Transport.HttpProxyTransport(options.httpProxy); + this._transport = new Transport.HttpProxyTransport(options.httpProxy, this._logger); } else { - this._transport = new Transport(); + this._transport = new Transport(this._logger); } }; diff --git a/lib/transport.js b/lib/transport.js index 9324f2c26..bd5d12b00 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -9,7 +9,8 @@ var inherits = require('inherits'), var request = require('requestretry'), canvas = require('./browser/canvas'), - jsonp = require('./browser/jsonp'); + jsonp = require('./browser/jsonp'), + Logger = require('./logger'); // set options if defaults setting is available in request, which is not available in xhr module. if (request.defaults) { @@ -66,7 +67,9 @@ function normalizeApiHost(apiHost) { * @class * @protected */ -var Transport = module.exports = function() {}; +var Transport = module.exports = function(logger) { + this._logger = logger || new Logger(); +}; /** * Make HTTP request, returns promise instead of stream @@ -89,6 +92,7 @@ Transport.prototype.httpRequest = function(params, callback) { }; req = httpRequest(opts, function(err, response) { if (response.attempts > 1){ + // TODO change this to use this._logger instead // just for tracking we're gonna log information about any retries we encounter // we're gonna do this as console to force it out as opposed to trying to pick up // on whatever mechanisms this module may already be doing to controle logging @@ -119,8 +123,9 @@ Transport.prototype._getHttpRequestModule = function() { * @extends Transport * @param {String} jsonpParam - Callback parameter name for JSONP invokation. */ -var JsonpTransport = Transport.JsonpTransport = function(jsonpParam) { +var JsonpTransport = Transport.JsonpTransport = function(jsonpParam, logger) { this._jsonpParam = jsonpParam; + this._logger = logger || new Logger(); }; inherits(JsonpTransport, Transport); @@ -140,8 +145,9 @@ JsonpTransport.supported = jsonp.supported; * @extends Transport * @param {Object} signedRequest - Parsed signed request object */ -var CanvasTransport = Transport.CanvasTransport = function(signedRequest) { +var CanvasTransport = Transport.CanvasTransport = function(signedRequest, logger) { this._signedRequest = signedRequest; + this._logger = logger || new Logger(); }; inherits(CanvasTransport, Transport); @@ -162,8 +168,9 @@ CanvasTransport.supported = canvas.supported; * @extends Transport * @param {String} proxyUrl - AJAX Proxy server URL */ -var ProxyTransport = Transport.ProxyTransport = function(proxyUrl) { +var ProxyTransport = Transport.ProxyTransport = function(proxyUrl, logger) { this._proxyUrl = proxyUrl; + this._logger = logger || new Logger(); }; inherits(ProxyTransport, Transport); @@ -208,8 +215,9 @@ ProxyTransport.prototype.httpRequest = function(params, callback) { * @extends Transport * @param {String} httpProxy - URL of the HTTP proxy server */ -var HttpProxyTransport = Transport.HttpProxyTransport = function(httpProxy) { +var HttpProxyTransport = Transport.HttpProxyTransport = function(httpProxy, logger) { this._httpProxy = httpProxy; + this._logger = logger || new Logger(); }; inherits(HttpProxyTransport, Transport); From d24632ea739482538fa7f869ac45ccdbf34cdf1f Mon Sep 17 00:00:00 2001 From: Matteo Zambon Date: Fri, 24 Sep 2021 10:10:42 +0100 Subject: [PATCH 2/5] Changed attempts log to use logger and to warn instead of info co-authored-by: beldougie --- lib/transport.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index bd5d12b00..7ebe7ac4e 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -82,6 +82,7 @@ Transport.prototype.httpRequest = function(params, callback) { var deferred = Promise.defer(); var req; var httpRequest = this._getHttpRequestModule(); + var logger = this._logger; var createRequest = function() { if (!req) { var opts = { @@ -92,11 +93,13 @@ Transport.prototype.httpRequest = function(params, callback) { }; req = httpRequest(opts, function(err, response) { if (response.attempts > 1){ - // TODO change this to use this._logger instead // just for tracking we're gonna log information about any retries we encounter // we're gonna do this as console to force it out as opposed to trying to pick up // on whatever mechanisms this module may already be doing to controle logging - console.log("JSForce performed %s attempts when making requests to %s", response.attempts, opts.url) + logger.log( + Logger.LogLevels.WARN, + 'JSForce performed ' + response.attempts + ' attempts when making requests to ' + opts.url + ); } if (err) { deferred.reject(err); From c47c8f9d3e141d1467af90c4d97d74fb27dc560e Mon Sep 17 00:00:00 2001 From: Matteo Zambon Date: Fri, 24 Sep 2021 10:11:27 +0100 Subject: [PATCH 3/5] Include WARN in console.error co-authored-by: beldougie --- lib/logger.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/logger.js b/lib/logger.js index d7e4e8caa..c91827c16 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -35,7 +35,7 @@ var LogLevels = Logger.LogLevels = { */ Logger.prototype.log = function(level, message) { if (this._logLevel <= level) { - if (level < LogLevels.ERROR) { + if (level < LogLevels.WARN) { console.log(message); } else { console.error(message); From 76d0e668e05f29accafee4a6fbb737c8fab66b38 Mon Sep 17 00:00:00 2001 From: Matteo Zambon Date: Fri, 24 Sep 2021 11:39:25 +0100 Subject: [PATCH 4/5] Log message as JSON with process and level co-authored-by: beldougie --- lib/logger.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/logger.js b/lib/logger.js index c91827c16..5c0d71fe1 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -27,6 +27,16 @@ var LogLevels = Logger.LogLevels = { "FATAL" : 5 }; +var logLevelNameByNumber = function (level) { + for (var k in LogLevels) { + if (LogLevels[k] == level) { + return k.toLowerCase(); + } + } + + return 'debug'; +} + /** * Output log * @@ -36,9 +46,9 @@ var LogLevels = Logger.LogLevels = { Logger.prototype.log = function(level, message) { if (this._logLevel <= level) { if (level < LogLevels.WARN) { - console.log(message); + console.log(JSON.stringify({process: 'jsforce', level: logLevelNameByNumber(level), message: message})); } else { - console.error(message); + console.error(JSON.stringify({process: 'jsforce', level: logLevelNameByNumber(level), message: message})); } } }; From 25ae01cdbdf1def6c39a4f99db869bdb515bc64f Mon Sep 17 00:00:00 2001 From: Matt Keeble Date: Fri, 17 Jun 2022 16:24:06 +0100 Subject: [PATCH 5/5] Update transport.js More safely handle the attempts tracking --- lib/transport.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index 7ebe7ac4e..9664e648f 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -92,10 +92,11 @@ Transport.prototype.httpRequest = function(params, callback) { retryStrategy: request.RetryStrategies.HTTPOrNetworkError }; req = httpRequest(opts, function(err, response) { - if (response.attempts > 1){ - // just for tracking we're gonna log information about any retries we encounter + const attempts = response ? response.attempts : err ? err.attempts : 0 + if (attempts > 1){ + // just for tracking we're going to log information about any retries we encounter // we're gonna do this as console to force it out as opposed to trying to pick up - // on whatever mechanisms this module may already be doing to controle logging + // on whatever mechanisms this module may already be doing to control logging logger.log( Logger.LogLevels.WARN, 'JSForce performed ' + response.attempts + ' attempts when making requests to ' + opts.url