From 64bd739a872006bc84099474fa6dfa1ddbbac30d Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 5 Feb 2020 10:05:36 +0000 Subject: [PATCH] Revert "Merge pull request #91 from overleaf/spd-trycatch-all-the-things" This reverts commit 2bf7f14f9d050c58f141f465633bb6e274b903dd, reversing changes made to 989240812532ca43a52513339f4dda8f44a80a64. --- .../coffee/DocumentUpdaterController.coffee | 17 +++------- .../real-time/app/coffee/DrainManager.coffee | 5 +-- services/real-time/app/coffee/Router.coffee | 34 +++++-------------- .../app/coffee/WebsocketLoadBalancer.coffee | 13 ++----- 4 files changed, 16 insertions(+), 53 deletions(-) diff --git a/services/real-time/app/coffee/DocumentUpdaterController.coffee b/services/real-time/app/coffee/DocumentUpdaterController.coffee index c96282f49e..2611d484ad 100644 --- a/services/real-time/app/coffee/DocumentUpdaterController.coffee +++ b/services/real-time/app/coffee/DocumentUpdaterController.coffee @@ -71,16 +71,10 @@ module.exports = DocumentUpdaterController = seen[client.id] = true if client.id == update.meta.source logger.log doc_id: doc_id, version: update.v, source: update.meta?.source, "distributing update to sender" - try - client.emit "otUpdateApplied", v: update.v, doc: update.doc - catch err - logger.warn client_id: client.id, doc_id: doc_id, err: err, "error sending update to sender" + client.emit "otUpdateApplied", v: update.v, doc: update.doc else if !update.dup # Duplicate ops should just be sent back to sending client for acknowledgement logger.log doc_id: doc_id, version: update.v, source: update.meta?.source, client_id: client.id, "distributing update to collaborator" - try - client.emit "otUpdateApplied", update - catch err - logger.warn client_id: client.id, doc_id: doc_id, err: err, "error sending update to collaborator" + client.emit "otUpdateApplied", update if Object.keys(seen).length < clientList.length metrics.inc "socket-io.duplicate-clients", 0.1 logger.log doc_id: doc_id, socketIoClients: (client.id for client in clientList), "discarded duplicate clients" @@ -88,10 +82,7 @@ module.exports = DocumentUpdaterController = _processErrorFromDocumentUpdater: (io, doc_id, error, message) -> for client in io.sockets.clients(doc_id) logger.warn err: error, doc_id: doc_id, client_id: client.id, "error from document updater, disconnecting client" - try - client.emit "otUpdateError", error, message - client.disconnect() - catch err - logger.warn client_id: client.id, doc_id: doc_id, err: err, cause: error, "error sending error to client" + client.emit "otUpdateError", error, message + client.disconnect() diff --git a/services/real-time/app/coffee/DrainManager.coffee b/services/real-time/app/coffee/DrainManager.coffee index 1a8fb73be1..2590a96726 100644 --- a/services/real-time/app/coffee/DrainManager.coffee +++ b/services/real-time/app/coffee/DrainManager.coffee @@ -30,10 +30,7 @@ module.exports = DrainManager = if !@RECONNECTED_CLIENTS[client.id] @RECONNECTED_CLIENTS[client.id] = true logger.log {client_id: client.id}, "Asking client to reconnect gracefully" - try - client.emit "reconnectGracefully" - catch err - logger.warn client_id: client.id, err: err, "error asking client to reconnect gracefully" + client.emit "reconnectGracefully" drainedCount++ haveDrainedNClients = (drainedCount == N) if haveDrainedNClients diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index d5a7472ab0..fcdb0c93dc 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -45,45 +45,29 @@ module.exports = Router = client?.on "error", (err) -> logger.err { clientErr: err }, "socket.io client error" if client.connected - try - client.emit("reconnectGracefully") - client.disconnect() - catch error - logger.warn error: error, cause: err, client_id: client.id, "error telling client to reconnect after error" + client.emit("reconnectGracefully") + client.disconnect() if settings.shutDownInProgress - try - client.emit("connectionRejected", {message: "retry"}) - client.disconnect() - catch error - logger.warn error: error, client_id: client.id, message: "retry", "error rejecting client connection" + client.emit("connectionRejected", {message: "retry"}) + client.disconnect() return if client? and error?.message?.match(/could not look up session by key/) logger.warn err: error, client: client?, session: session?, "invalid session" # tell the client to reauthenticate if it has an invalid session key - try - client.emit("connectionRejected", {message: "invalid session"}) - client.disconnect() - catch error - logger.warn error: error, client_id: client.id, message: "invalid session", "error rejecting client connection" + client.emit("connectionRejected", {message: "invalid session"}) + client.disconnect() return if error? logger.err err: error, client: client?, session: session?, "error when client connected" - try - client?.emit("connectionRejected", {message: "error"}) - client?.disconnect() - catch error - logger.warn error: error, client_id: client?.id, message: "error", "error rejecting client connection" + client?.emit("connectionRejected", {message: "error"}) + client?.disconnect() return # send positive confirmation that the client has a valid connection - try - client.emit("connectionAccepted") - catch error - logger.warn error: error, client_id: client.id, "error accepting client connection" - return + client.emit("connectionAccepted") metrics.inc('socket-io.connection') metrics.gauge('socket-io.clients', io.sockets.clients()?.length) diff --git a/services/real-time/app/coffee/WebsocketLoadBalancer.coffee b/services/real-time/app/coffee/WebsocketLoadBalancer.coffee index 64274cc2bd..12b7ef812b 100644 --- a/services/real-time/app/coffee/WebsocketLoadBalancer.coffee +++ b/services/real-time/app/coffee/WebsocketLoadBalancer.coffee @@ -67,10 +67,7 @@ module.exports = WebsocketLoadBalancer = logger.error {err: error, channel}, "error parsing JSON" return if message.room_id == "all" - try - io.sockets.emit(message.message, message.payload...) - catch error - logger.warn message: message, error: error, "error sending message to clients" + io.sockets.emit(message.message, message.payload...) else if message.message is 'clientTracking.refresh' && message.room_id? clientList = io.sockets.clients(message.room_id) logger.log {channel:channel, message: message.message, room_id: message.room_id, message_id: message._id, socketIoClients: (client.id for client in clientList)}, "refreshing client list" @@ -99,16 +96,10 @@ module.exports = WebsocketLoadBalancer = , (client, cb) -> Utils.getClientAttributes client, ['is_restricted_user'], (err, {is_restricted_user}) -> return cb(err) if err? - if client.disconnected - logger.warn {channel:channel, client: client.id}, "skipping emit, client not connected" - return cb() if !seen[client.id] seen[client.id] = true if !(is_restricted_user && message.message not in RESTRICTED_USER_MESSAGE_TYPE_PASS_LIST) - try - client.emit(message.message, message.payload...) - catch error - console.log(message: message, client_id: client.id, error: error, "error sending message to client") + client.emit(message.message, message.payload...) cb() , (err) -> if err?