From 6ed2a0d04d56197f97fd875fae6d8709f9eb2c16 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 13 Nov 2014 12:03:43 +0000 Subject: [PATCH] Refactor client attribute fetching and logging --- services/real-time/app/coffee/Router.coffee | 38 ++++++------------- .../app/coffee/WebsocketController.coffee | 15 +++----- 2 files changed, 16 insertions(+), 37 deletions(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index b3626f4617..58baa28af9 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -2,26 +2,14 @@ Metrics = require "metrics-sharelatex" logger = require "logger-sharelatex" WebsocketController = require "./WebsocketController" HttpController = require "./HttpController" +Utils = require "./Utils" module.exports = Router = - # We don't want to send raw errors back to the client, in case they - # contain sensitive data. Instead we log them out, and send a generic - # JSON object which can be serialized over socket.io - _createCallbackWithErrorFilter: (client, method, callback) -> - return (err, args...) -> - if err? - - err = {message: "Something went wrong"} - callback err, args... - - # Used in error reporting - _getClientData: (client, callback = (error, data) ->) -> - client.get "user_id", (error, user_id) -> - client.get "project_id", (error, project_id) -> - client.get "doc_id", (error, doc_id) -> - callback null, { id: client.id, user_id, project_id, doc_id } - configure: (app, io, session) -> + app.set("io", io) + app.get "/clients", HttpController.getConnectedClients + app.get "/clients/:client_id", HttpController.getConnectedClient + session.on 'connection', (error, client, session) -> if error? logger.err err: error, "error when client connected" @@ -41,8 +29,7 @@ module.exports = Router = client.on "joinProject", (data = {}, callback) -> WebsocketController.joinProject client, user, data.project_id, (err, args...) -> if err? - Router._getClientData client, (_, client) -> - logger.error {err, client, project_id: data.project_id}, "server side error in joinProject" + logger.error {err, user_id: user?.id, project_id: data.project_id}, "server side error in joinProject" # Don't return raw error to prevent leaking server side info return callback {message: "Something went wrong"} else @@ -57,8 +44,8 @@ module.exports = Router = WebsocketController.joinDoc client, doc_id, fromVersion, (err, args...) -> if err? - Router._getClientData client, (_, client) -> - logger.error {err, client, doc_id, fromVersion}, "server side error in joinDoc" + Utils.getClientAttributes client, ["project_id", "user_id"], (_, {project_id, user_id}) -> + logger.error {err, client_id: client.id, user_id, project_id, doc_id, fromVersion}, "server side error in joinDoc" # Don't return raw error to prevent leaking server side info return callback {message: "Something went wrong"} else @@ -67,13 +54,10 @@ module.exports = Router = client.on "leaveDoc", (doc_id, callback) -> WebsocketController.leaveDoc client, doc_id, (err, args...) -> if err? - Router._getClientData client, (_, client) -> - logger.error {err, client, doc_id}, "server side error in leaveDoc" + Utils.getClientAttributes client, ["project_id", "user_id"], (_, {project_id, user_id}) -> + logger.error {err, client_id: client.id, user_id, project_id, doc_id}, "server side error in leaveDoc" # Don't return raw error to prevent leaking server side info return callback {message: "Something went wrong"} else callback(null, args...) - - app.set("io", io) - app.get "/clients", HttpController.getConnectedClients - app.get "/clients/:client_id", HttpController.getConnectedClient \ No newline at end of file + \ No newline at end of file diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index cd0e2a14dd..30ea76bebf 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -2,6 +2,7 @@ logger = require "logger-sharelatex" WebApiManager = require "./WebApiManager" AuthorizationManager = require "./AuthorizationManager" DocumentUpdaterManager = require "./DocumentUpdaterManager" +Utils = require "./Utils" module.exports = WebsocketController = # If the protocol version changes when the client reconnects, @@ -36,8 +37,8 @@ module.exports = WebsocketController = callback null, project, privilegeLevel, WebsocketController.PROTOCOL_VERSION joinDoc: (client, doc_id, fromVersion = -1, callback = (error, doclines, version, ops) ->) -> - WebsocketController._getClientData client, (error, {client_id, user_id, project_id}) -> - logger.log {user_id, project_id, doc_id, fromVersion, client_id}, "client joining doc" + Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) -> + logger.log {user_id, project_id, doc_id, fromVersion, client_id: client.id}, "client joining doc" AuthorizationManager.assertClientCanViewProject client, (error) -> return callback(error) if error? @@ -60,13 +61,7 @@ module.exports = WebsocketController = callback null, escapedLines, version, ops leaveDoc: (client, doc_id, callback = (error) ->) -> - WebsocketController._getClientData client, (error, {client_id, user_id, project_id}) -> - logger.log {user_id, project_id, doc_id, client_id}, "client leaving doc" + Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) -> + logger.log {user_id, project_id, doc_id, client_id: client.id}, "client leaving doc" client.leave doc_id callback() - - # Only used in logging. - _getClientData: (client, callback = (error, data) ->) -> - client.get "user_id", (error, user_id) -> - client.get "project_id", (error, project_id) -> - callback null, {client_id: client.id, project_id, user_id} \ No newline at end of file