Skip to content

Commit

Permalink
EZP-29253: Make session refresh re-try on timeouts and server errors (e…
Browse files Browse the repository at this point in the history 
…zsystems#90)

* EZP-29253: Make session refresh re-try on timouts and server errors

In order to try to improve stability of rest client, re-try session
refresh which is often the one helding others back on timout and 5xx errors.

* Change to be time based and increase timout to 180sec, also add callback for re-try

* re-build
  • Loading branch information
@andrerom
andrerom committed Jun 11, 2018
1 parent d2eb0e5 commit 4624430
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 25 deletions.
2 changes: 1 addition & 1 deletion dist/CAPI-min.js
View file

Large diffs are not rendered by default.

62 changes: 50 additions & 12 deletions dist/CAPI.js
View file
Expand Up @@ -1136,6 +1136,7 @@ define('ConnectionManager',["structures/Response", "structures/Request", "struct
* @param [headers={}] {object} object literal describing request headers
* @param [requestEventHandlers] {Object} a set of callbacks to apply on a specific XHR event like onload, onerror, onprogress, etc.
* @param callback {Function} function, which will be executed on request success
* @param timeout {Number} Optionally the number of milliseconds to set as timeout, by default none set.
* @example
* var connectionManager = jsCAPI.getConnectionManager();
*
Expand Down Expand Up @@ -1165,7 +1166,7 @@ define('ConnectionManager',["structures/Response", "structures/Request", "struct
* callback
* );
*/
ConnectionManager.prototype.request = function (method, url, body, headers, requestEventHandlers, callback) {
ConnectionManager.prototype.request = function (method, url, body, headers, requestEventHandlers, callback, timeout) {
var that = this,
request,
nextRequest,
Expand Down Expand Up @@ -1219,7 +1220,8 @@ define('ConnectionManager',["structures/Response", "structures/Request", "struct
method : method,
url : this._endPointUrl + url,
body : body,
headers : headers
headers : headers,
timeout : timeout
});

// Requests suspending workflow
Expand Down Expand Up @@ -1452,6 +1454,7 @@ define('connections/XmlHttpRequestConnection',["structures/Response", "structure
);
return;
}

callback(false, response);
};

Expand Down Expand Up @@ -1487,6 +1490,10 @@ define('connections/XmlHttpRequestConnection',["structures/Response", "structure
XHR.open(method, request.url, true);
}

if (request.timeout) {
XHR.timeout = request.timeout;// time in milliseconds, e.g. 62000 == 62 seconds
}

if (method !== request.method) {
XHR.setRequestHeader("X-HTTP-Method-Override", request.method);
}
Expand Down Expand Up @@ -7945,9 +7952,44 @@ define('services/UserService',['structures/SessionCreateStruct', 'structures/Use
* @method refreshSession
* @param {String} sessionId the session identifier (e.g. "o7i8r1sapfc9r84ae53bgq8gp4")
* @param {Function} callback
*/
UserService.prototype.refreshSession = function (sessionId, callback) {
var that = this;
* @param {Number} timeout Optional timeout in milliseconds, default is 180000 (180sec, as in 3 minutes)
* @param {Function} reTryCallback Optional callback to get notified if code is retrying session refresh
* NOTE: gets called every time, except on the first try!
*/
UserService.prototype.refreshSession = function (sessionId, callback, timeout, reTryCallback) {
var that = this, firstTry = true, date = new Date(), refreshSessionInfo, sessionRefresh = function(e, result) {
// We re-try session refresh for 180 seconds on timeouts/server errors as it is often the requests holding
// others back(see ConnectionManager), and is thus the one request we typically need to re-try a few
// times if backend has temporary downtime or is undergoing brief maintenance.
if ((new Date()) - date >= (timeout || 180000)) {
callback(e ? e : true, result);
return;
}

if (firstTry) {
firstTry = false;
} else if (reTryCallback) {
reTryCallback();
}

that._connectionManager.request(
"POST",
parseUriTemplate(refreshSessionInfo._href, {sessionId: sessionId}),
"",
{"Accept": refreshSessionInfo["_media-type"]},
{"ontimeout": sessionRefresh},
function (error, result) {
// Re-try refresh on unexpected 5xx errors
if (error && result.status >= 500) {
sessionRefresh(error, result);
return;
}

callback(error, result);
},
30000// 30sec timeout, to avoid having to wait for default timeout of 600s, better to re-try if no reply
);
};

this._discoveryService.getInfoObject(
"refreshSession",
Expand All @@ -7956,13 +7998,9 @@ define('services/UserService',['structures/SessionCreateStruct', 'structures/Use
callback(error, refreshSession);
return;
}
that._connectionManager.request(
"POST",
parseUriTemplate(refreshSession._href, {sessionId: sessionId}),
"",
{"Accept": refreshSession["_media-type"]},
callback
);

refreshSessionInfo = refreshSession;
sessionRefresh();
}
);
};
Expand Down
6 changes: 4 additions & 2 deletions src/ConnectionManager.js
View file
Expand Up @@ -35,6 +35,7 @@ define(["structures/Response", "structures/Request", "structures/CAPIError"],
* @param [headers={}] {object} object literal describing request headers
* @param [requestEventHandlers] {Object} a set of callbacks to apply on a specific XHR event like onload, onerror, onprogress, etc.
* @param callback {Function} function, which will be executed on request success
* @param timeout {Number} Optionally the number of milliseconds to set as timeout, by default none set.
* @example
* var connectionManager = jsCAPI.getConnectionManager();
*
Expand Down Expand Up @@ -64,7 +65,7 @@ define(["structures/Response", "structures/Request", "structures/CAPIError"],
* callback
* );
*/
ConnectionManager.prototype.request = function (method, url, body, headers, requestEventHandlers, callback) {
ConnectionManager.prototype.request = function (method, url, body, headers, requestEventHandlers, callback, timeout) {
var that = this,
request,
nextRequest,
Expand Down Expand Up @@ -118,7 +119,8 @@ define(["structures/Response", "structures/Request", "structures/CAPIError"],
method : method,
url : this._endPointUrl + url,
body : body,
headers : headers
headers : headers,
timeout : timeout
});

// Requests suspending workflow
Expand Down
5 changes: 5 additions & 0 deletions src/connections/XmlHttpRequestConnection.js
View file
Expand Up @@ -60,6 +60,7 @@ define(["structures/Response", "structures/CAPIError"], function (Response, CAPI
);
return;
}

callback(false, response);
};

Expand Down Expand Up @@ -95,6 +96,10 @@ define(["structures/Response", "structures/CAPIError"], function (Response, CAPI
XHR.open(method, request.url, true);
}

if (request.timeout) {
XHR.timeout = request.timeout;// time in milliseconds, e.g. 62000 == 62 seconds
}

if (method !== request.method) {
XHR.setRequestHeader("X-HTTP-Method-Override", request.method);
}
Expand Down
49 changes: 40 additions & 9 deletions src/services/UserService.js
View file
Expand Up @@ -1319,9 +1319,44 @@ define(['structures/SessionCreateStruct', 'structures/UserCreateStruct', 'struct
* @method refreshSession
* @param {String} sessionId the session identifier (e.g. "o7i8r1sapfc9r84ae53bgq8gp4")
* @param {Function} callback
* @param {Number} timeout Optional timeout in milliseconds, default is 180000 (180sec, as in 3 minutes)
* @param {Function} reTryCallback Optional callback to get notified if code is retrying session refresh
* NOTE: gets called every time, except on the first try!
*/
UserService.prototype.refreshSession = function (sessionId, callback) {
var that = this;
UserService.prototype.refreshSession = function (sessionId, callback, timeout, reTryCallback) {
var that = this, firstTry = true, date = new Date(), refreshSessionInfo, sessionRefresh = function(e, result) {
// We re-try session refresh for 180 seconds on timeouts/server errors as it is often the requests holding
// others back(see ConnectionManager), and is thus the one request we typically need to re-try a few
// times if backend has temporary downtime or is undergoing brief maintenance.
if ((new Date()) - date >= (timeout || 180000)) {
callback(e ? e : true, result);
return;
}

if (firstTry) {
firstTry = false;
} else if (reTryCallback) {
reTryCallback();
}

that._connectionManager.request(
"POST",
parseUriTemplate(refreshSessionInfo._href, {sessionId: sessionId}),
"",
{"Accept": refreshSessionInfo["_media-type"]},
{"ontimeout": sessionRefresh},
function (error, result) {
// Re-try refresh on unexpected 5xx errors
if (error && result.status >= 500) {
sessionRefresh(error, result);
return;
}

callback(error, result);
},
30000// 30sec timeout, to avoid having to wait for default timeout of 600s, better to re-try if no reply
);
};

this._discoveryService.getInfoObject(
"refreshSession",
Expand All @@ -1330,13 +1365,9 @@ define(['structures/SessionCreateStruct', 'structures/UserCreateStruct', 'struct
callback(error, refreshSession);
return;
}
that._connectionManager.request(
"POST",
parseUriTemplate(refreshSession._href, {sessionId: sessionId}),
"",
{"Accept": refreshSession["_media-type"]},
callback
);

refreshSessionInfo = refreshSession;
sessionRefresh();
}
);
};
Expand Down
4 changes: 3 additions & 1 deletion test/UserService.tests.js
View file
Expand Up @@ -1403,7 +1403,9 @@ define(function (require) {
parseUriTemplate(testRefreshSession, {sessionId: sessionId}),
"",
{"Accept": refreshSessionInfo["_media-type"]},
mockCallback
{"ontimeout": jasmine.any(Function)},
jasmine.any(Function),
30000
);
});

Expand Down

0 comments on commit 4624430

Please sign in to comment.