Skip to content

Commit

Permalink
Report XMPP errors to the backend.
Browse files Browse the repository at this point in the history
This CL reports the XMPP Error to our logging backends

BUG=509070

Review URL: https://codereview.chromium.org/1289413002

Cr-Commit-Position: refs/heads/master@{#344130}
  • Loading branch information
kelvinp authored and Commit bot committed Aug 19, 2015
1 parent cb5c166 commit 6c4f3d7
Show file tree
Hide file tree
Showing 13 changed files with 349 additions and 24 deletions.
2 changes: 2 additions & 0 deletions remoting/remoting_webapp_files.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
'webapp/base/js/xhr_event_writer_unittest.js',
'webapp/base/js/xhr_unittest.js',
'webapp/base/js/xmpp_connection_unittest.js',
'webapp/base/js/xmpp_error_cache_unittest.js',
'webapp/base/js/xmpp_login_handler_unittest.js',
'webapp/base/js/xmpp_stream_parser_unittest.js',
'webapp/crd/js/apps_v2_migration_unittest.js',
Expand Down Expand Up @@ -179,6 +180,7 @@
'webapp/base/js/host_desktop.js',
'webapp/base/js/smart_reconnector.js',
'webapp/base/js/telemetry_event_writer.js',
'webapp/base/js/xmpp_error_cache.js',
],
# Remoting core JavaScript files.
'remoting_webapp_shared_js_core_files': [
Expand Down
17 changes: 16 additions & 1 deletion remoting/webapp/base/js/chromoting_event.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ remoting.ChromotingEvent = function(type) {
this.signal_strategy_type;
/** @type {remoting.ChromotingEvent.SignalStrategyProgress} */
this.signal_strategy_progress;
/** @type {?remoting.ChromotingEvent.XmppError} */
this.xmpp_error;

this.init_();
};
Expand Down Expand Up @@ -122,6 +124,20 @@ remoting.ChromotingEvent.isEndOfSession = function(event) {
return endStates.indexOf(event.session_state) !== -1;
};

/**
* This is declared as a separate structure to match the proto format
* on the cloud. The cloud will parse the raw stanza for more detailed
* fields, e.g. error condition, error type, jingle action, etc.
*
* @param {string} stanza
* @struct
* @constructor
*/
remoting.ChromotingEvent.XmppError = function(stanza) {
/** @type {string} */
this.raw_stanza = stanza;
};

})();

/**
Expand Down Expand Up @@ -220,4 +236,3 @@ remoting.ChromotingEvent.SignalStrategyProgress = {
SUCCEEDED_LATE: 4,
FAILED_LATE: 5
};

7 changes: 6 additions & 1 deletion remoting/webapp/base/js/client_session.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ remoting.ClientSession = function(
/** @private {remoting.FormatIq} */
this.iqFormatter_ = null;

/** @private {remoting.XmppErrorCache} */
this.xmppErrorCache_ = new remoting.XmppErrorCache();

/**
* Allow host-offline error reporting to be suppressed in situations where it
* would not be useful, for example, when using a cached host JID.
Expand Down Expand Up @@ -449,6 +452,7 @@ remoting.ClientSession.prototype.onIncomingMessage_ = function(message) {
var formatted = new XMLSerializer().serializeToString(message);
console.log(base.timestamp() +
this.iqFormatter_.prettifyReceiveIq(formatted));
this.xmppErrorCache_.processStanza(message);
this.plugin_.onIncomingIq(formatted);
};

Expand Down Expand Up @@ -562,7 +566,8 @@ remoting.ClientSession.prototype.setState_ = function(newState) {
}

this.notifyStateChanges_(oldState, this.state_);
this.logger_.logClientSessionStateChange(this.state_, this.error_);
this.logger_.logClientSessionStateChange(
this.state_, this.error_, this.xmppErrorCache_.getFirstError());
};

/**
Expand Down
5 changes: 4 additions & 1 deletion remoting/webapp/base/js/log_to_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ remoting.LogToServer.SESSION_ID_LEN_ = 20;
*
* @param {remoting.ClientSession.State} state
* @param {!remoting.Error} connectionError
* @param {?remoting.ChromotingEvent.XmppError} xmppError
*/
remoting.LogToServer.prototype.logClientSessionStateChange =
function(state, connectionError) {
function(state, connectionError, xmppError) {
this.maybeExpireSessionId_();
// Log the session state change.
var entry = remoting.ServerLogEntry.makeClientSessionStateChange(
Expand All @@ -67,6 +68,8 @@ remoting.LogToServer.prototype.logClientSessionStateChange =
entry.addChromeVersionField();
entry.addWebappVersionField();
entry.addSessionIdField(this.sessionId_);
entry.addXmppError(xmppError);

this.log_(entry);
// Don't accumulate connection statistics across state changes.
this.logAccumulatedStatistics_();
Expand Down
5 changes: 4 additions & 1 deletion remoting/webapp/base/js/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ remoting.Logger.prototype.logSignalStrategyProgress =
*
* @param {remoting.ClientSession.State} state
* @param {!remoting.Error} connectionError
* @param {?remoting.ChromotingEvent.XmppError} xmppError The XMPP error
* as described in http://xmpp.org/rfcs/rfc6120.html#stanzas-error.
* Set if the connecton error originates from the an XMPP stanza error.
*/
remoting.Logger.prototype.logClientSessionStateChange =
function(state, connectionError) {};
function(state, connectionError, xmppError) {};

/**
* Logs connection statistics.
Expand Down
15 changes: 15 additions & 0 deletions remoting/webapp/base/js/server_log_entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ remoting.ServerLogEntry.KEY_SIGNAL_STRATEGY_PROGRESS_ =
/** @private */
remoting.ServerLogEntry.KEY_SESSION_DURATION_SECONDS_ = 'session-duration';

/** @private */
remoting.ServerLogEntry.KEY_XMPP_ERROR_RAW_STANZA = 'xmpp-error-raw-stanza';

/**
* @private
Expand Down Expand Up @@ -440,3 +442,16 @@ remoting.ServerLogEntry.prototype.addModeField = function(mode) {
remoting.ServerLogEntry.prototype.addApplicationId = function() {
this.set_(remoting.ServerLogEntry.KEY_APP_ID_, chrome.runtime.id);
};

/**
* Adds a field specifying the XMPP error to this log entry.
* @param {?remoting.ChromotingEvent.XmppError} xmppError
* @return {void} Nothing.
*/
remoting.ServerLogEntry.prototype.addXmppError = function(xmppError) {
if (!Boolean(xmppError)) {
return;
}
this.set_(remoting.ServerLogEntry.KEY_XMPP_ERROR_RAW_STANZA,
xmppError.raw_stanza);
};
13 changes: 10 additions & 3 deletions remoting/webapp/base/js/session_logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,11 @@ remoting.SessionLogger.prototype.logSignalStrategyProgress =

/** @override {remoting.Logger} */
remoting.SessionLogger.prototype.logClientSessionStateChange =
function(state, connectionError) {
function(state, connectionError, xmppError) {
this.maybeExpireSessionId_();
// Log the session state change.
var entry = this.makeSessionStateChange_(state, connectionError);
var entry =
this.makeSessionStateChange_(state, connectionError, xmppError);
this.log_(entry);
// Don't accumulate connection statistics across state changes.
this.logAccumulatedStatistics_();
Expand All @@ -111,15 +112,21 @@ remoting.SessionLogger.prototype.logStatistics = function(stats) {
/**
* @param {remoting.ClientSession.State} state
* @param {remoting.Error} error
* @param {?remoting.ChromotingEvent.XmppError} xmppError
* @return {remoting.ChromotingEvent}
* @private
*/
remoting.SessionLogger.prototype.makeSessionStateChange_ =
function(state, error) {
function(state, error, xmppError) {
var entry = new remoting.ChromotingEvent(
remoting.ChromotingEvent.Type.SESSION_STATE);
entry.connection_error = toConnectionError(error);
entry.session_state = toSessionState(state);

if (Boolean(xmppError)) {
entry.xmpp_error = xmppError;
}

this.fillEvent_(entry);
return entry;
};
Expand Down
47 changes: 43 additions & 4 deletions remoting/webapp/base/js/session_logger_unittest.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ function verifyEvent(assert, index, expected) {
var event = /** @type {Object} */ (logWriterSpy.getCall(index).args[0]);

for (var key in expected) {
assert.equal(event[key], expected[key], 'Verifying ChromotingEvent.' + key);
assert.deepEqual(event[key], expected[key],
'Verifying ChromotingEvent.' + key);
}
}

Expand Down Expand Up @@ -83,7 +84,7 @@ QUnit.test('logClientSessionStateChange()', function(assert){

logger.logClientSessionStateChange(
remoting.ClientSession.State.FAILED,
new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE));
new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE), null);
var sessionId = logger.getSessionId();

assert.ok(sessionId !== null);
Expand All @@ -105,6 +106,44 @@ QUnit.test('logClientSessionStateChange()', function(assert){
});
});

QUnit.test('logClientSessionStateChange() should handle XMPP error',
function(assert){
var Event = remoting.ChromotingEvent;

logger = new remoting.SessionLogger(Event.Role.CLIENT, logWriter);
logger.setLogEntryMode(Event.Mode.ME2ME);
logger.setConnectionType('stun');
logger.setHostVersion('host_version');

var xmppError = new remoting.ChromotingEvent.XmppError('<fake-stanza/>');

logger.logClientSessionStateChange(
remoting.ClientSession.State.FAILED,
new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE), xmppError);
var sessionId = logger.getSessionId();

assert.ok(sessionId !== null);

verifyEvent(assert, 0, {
type: Event.Type.SESSION_STATE,
session_state: Event.SessionState.CONNECTION_FAILED,
connection_error: Event.ConnectionError.HOST_OFFLINE,
os: Event.Os.MAC,
os_version: '10.9.5',
cpu: 'Intel',
browser_version: '43.0.2357.81',
application_id: 'extensionId',
role: Event.Role.CLIENT,
mode: Event.Mode.ME2ME,
connection_type: Event.ConnectionType.STUN,
host_version: 'host_version',
session_id: sessionId,
xmpp_error: {
raw_stanza: '<fake-stanza/>'
}
});
});

QUnit.test('logClientSessionStateChange() should handle sessionId change.',
function(assert){
var clock = sinon.useFakeTimers();
Expand All @@ -122,7 +161,7 @@ QUnit.test('logClientSessionStateChange() should handle sessionId change.',

// Logs the event.
logger.logClientSessionStateChange(
remoting.ClientSession.State.AUTHENTICATED, remoting.Error.none());
remoting.ClientSession.State.AUTHENTICATED, remoting.Error.none(), null);

var newSessionId = logger.getSessionId();
verifyEvent(assert, 0, {
Expand Down Expand Up @@ -185,7 +224,7 @@ QUnit.test('logClientSessionStateChange() should log session_duration.',

// Logs the event.
logger.logClientSessionStateChange(
remoting.ClientSession.State.CONNECTED, remoting.Error.none());
remoting.ClientSession.State.CONNECTED, remoting.Error.none(), null);

verifyEvent(assert, 0, {
type: Event.Type.SESSION_STATE,
Expand Down
10 changes: 5 additions & 5 deletions remoting/webapp/base/js/telemetry_event_writer_unittest.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ QUnit.test('should send CANCELED event when window is closed while connecting.',
chrome.app.window.current().id = 'fake-window-id';
}).then(function() {
logger.logClientSessionStateChange(
remoting.ClientSession.State.CONNECTING, remoting.Error.none());
remoting.ClientSession.State.CONNECTING, remoting.Error.none(), null);
}).then(function() {
return service.unbindSession('fake-window-id');
}).then(function() {
Expand All @@ -116,10 +116,10 @@ QUnit.test('should send CLOSED event when window is closed while connected.',
chrome.app.window.current().id = 'fake-window-id';
}).then(function() {
logger.logClientSessionStateChange(
remoting.ClientSession.State.CONNECTING, remoting.Error.none());
remoting.ClientSession.State.CONNECTING, remoting.Error.none(), null);
}).then(function() {
logger.logClientSessionStateChange(
remoting.ClientSession.State.CONNECTED, remoting.Error.none());
remoting.ClientSession.State.CONNECTED, remoting.Error.none(), null);
}).then(function() {
return service.unbindSession('fake-window-id');
}).then(function() {
Expand Down Expand Up @@ -147,10 +147,10 @@ QUnit.test('should not send CLOSED event when window is closed unconnected.',
chrome.app.window.current().id = 'fake-window-id';
}).then(function() {
logger.logClientSessionStateChange(
remoting.ClientSession.State.CONNECTING, remoting.Error.none());
remoting.ClientSession.State.CONNECTING, remoting.Error.none(), null);
}).then(function() {
logger.logClientSessionStateChange(
remoting.ClientSession.State.FAILED, remoting.Error.none());
remoting.ClientSession.State.FAILED, remoting.Error.none(), null);
}).then(function() {
return service.unbindSession('fake-window-id');
}).then(function() {
Expand Down
Loading

0 comments on commit 6c4f3d7

Please sign in to comment.