From 6656a89092a073899630e2994d93ae1cb705f188 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Wed, 13 Mar 2019 12:05:47 +1100 Subject: [PATCH 1/2] Cleaned up a lot of the logs, stop them from printing more than once etc --- js/modules/loki_message_api.js | 57 +++++++++++++--------------------- js/modules/loki_snode_api.js | 33 +++++++------------- libloki/api.js | 10 +----- libtextsecure/errors.js | 8 ++--- 4 files changed, 35 insertions(+), 73 deletions(-) diff --git a/js/modules/loki_message_api.js b/js/modules/loki_message_api.js index 36d71c306..4ec1b1b41 100644 --- a/js/modules/loki_message_api.js +++ b/js/modules/loki_message_api.js @@ -138,14 +138,20 @@ class LokiMessageAPI { }); lokiP2pAPI.setContactOnline(pubKey); window.Whisper.events.trigger('p2pMessageSent', messageEventData); + if (isPing) { + log.info(`Successfully pinged ${pubKey}`); + } else { + log.info(`Successful p2p message to ${pubKey}`); + } return; } catch (e) { - log.warn('Failed to send P2P message, falling back to storage', e); lokiP2pAPI.setContactOffline(pubKey); if (isPing) { // If this was just a ping, we don't bother sending to storage server + log.warn('Ping failed, contact marked offline', e); return; } + log.warn('Failed to send P2P message, falling back to storage', e); } } @@ -194,16 +200,10 @@ class LokiMessageAPI { nodeComplete(nodeUrl); successfulRequests += 1; } catch (e) { - log.warn('Send message error:', e); + log.warn('Loki send message:', e); if (e instanceof NotFoundError) { canResolve = false; } else if (e instanceof HTTPError) { - log.error( - `POST ${e.response.url} (store)`, - e.response.status, - 'Error sending message' - ); - // We mark the node as complete as we could still reach it nodeComplete(nodeUrl); } else { @@ -212,7 +212,7 @@ class LokiMessageAPI { nodeUrl ); if (removeNode) { - log.error('Loki SendMessages:', e); + log.error('Loki send message:', e); nodeComplete(nodeUrl); failedNodes.push(nodeUrl); } @@ -232,11 +232,12 @@ class LokiMessageAPI { if (swarmNodes.length === 0) { if (successfulRequests !== 0) { // TODO: Decide how to handle some completed requests but not enough + log.warn(`Partially successful storage message to ${pubKey}`); return; } throw new window.textsecure.EmptySwarmError( pubKey, - new Error('Ran out of swarm nodes to query') + 'Ran out of swarm nodes to query' ); } } @@ -250,6 +251,7 @@ class LokiMessageAPI { .map(nodeUrl => doRequest(nodeUrl)) ); } + log.info(`Successful storage message to ${pubKey}`); } async retrieveMessages(callback) { @@ -258,12 +260,7 @@ class LokiMessageAPI { let canResolve = true; let successfulRequests = 0; - let ourSwarmNodes; - try { - ourSwarmNodes = await lokiSnodeAPI.getOurSwarmNodes(); - } catch (e) { - throw new window.textsecure.EmptySwarmError(ourKey, e); - } + let ourSwarmNodes = await lokiSnodeAPI.getOurSwarmNodes(); const nodeComplete = nodeUrl => { completedNodes.push(nodeUrl); @@ -292,16 +289,10 @@ class LokiMessageAPI { } successfulRequests += 1; } catch (e) { - log.warn('Retrieve message error:', e); + log.warn('Loki retrieve messages:', e); if (e instanceof NotFoundError) { canResolve = false; } else if (e instanceof HTTPError) { - log.error( - `POST ${e.response.url} (retrieve)`, - e.response.status, - `Error retrieving messages from ${nodeUrl}` - ); - // We mark the node as complete as we could still reach it nodeComplete(nodeUrl); } else { @@ -310,7 +301,7 @@ class LokiMessageAPI { nodeUrl ); if (removeNode) { - log.error('Loki RetrieveMessages:', e); + log.error('Loki retrieve messages:', e); nodeComplete(nodeUrl); } } @@ -322,24 +313,18 @@ class LokiMessageAPI { throw new window.textsecure.DNSResolutionError('Retrieving messages'); } if (Object.keys(ourSwarmNodes).length === 0) { - try { - ourSwarmNodes = await lokiSnodeAPI.getOurSwarmNodes(); - // Filter out the nodes we have already got responses from - completedNodes.forEach(nodeUrl => delete ourSwarmNodes[nodeUrl]); - } catch (e) { - throw new window.textsecure.EmptySwarmError( - window.textsecure.storage.user.getNumber(), - e - ); - } + ourSwarmNodes = await lokiSnodeAPI.getOurSwarmNodes(); + // Filter out the nodes we have already got responses from + completedNodes.forEach(nodeUrl => delete ourSwarmNodes[nodeUrl]); + if (Object.keys(ourSwarmNodes).length === 0) { if (successfulRequests !== 0) { // TODO: Decide how to handle some completed requests but not enough return; } throw new window.textsecure.EmptySwarmError( - window.textsecure.storage.user.getNumber(), - new Error('Ran out of swarm nodes to query') + ourKey, + 'Ran out of swarm nodes to query' ); } } diff --git a/js/modules/loki_snode_api.js b/js/modules/loki_snode_api.js index 9f805bb8b..e24be4426 100644 --- a/js/modules/loki_snode_api.js +++ b/js/modules/loki_snode_api.js @@ -1,5 +1,5 @@ /* eslint-disable class-methods-use-this */ -/* global log, window, ConversationController */ +/* global window, ConversationController */ const fetch = require('node-fetch'); const is = require('@sindresorhus/is'); @@ -148,9 +148,6 @@ class LokiSnodeAPI { // Try refresh our swarm list once const ourKey = window.textsecure.storage.user.getNumber(); const nodeAddresses = await this.getSwarmNodes(ourKey); - if (!nodeAddresses || nodeAddresses.length === 0) { - throw Error('Could not load our swarm'); - } nodeAddresses.forEach(url => { this.ourSwarmNodes[url] = { @@ -174,7 +171,6 @@ class LokiSnodeAPI { newSwarmNodes = await this.getSwarmNodes(pubKey); } catch (e) { // TODO: Handle these errors sensibly - log.error(e); newSwarmNodes = []; } resolve(newSwarmNodes); @@ -218,7 +214,10 @@ class LokiSnodeAPI { try { response = await fetch(options.url, fetchOptions); } catch (e) { - throw HTTPError('getSwarmNodes fetch error', 0, e.toString()); + throw new window.textsecure.EmptySwarmError( + pubKey, + 'Could not retrieve swarm nodes' + ); } let result; @@ -233,25 +232,15 @@ class LokiSnodeAPI { result = await response.text(); } - if (response.status >= 0 && response.status < 400) { - return result.nodes; + if (response.status !== 200 || !result.nodes || result.nodes === []) { + throw new window.textsecure.EmptySwarmError( + pubKey, + 'Could not retrieve swarm nodes' + ); } - throw HTTPError('getSwarmNodes: error response', response.status, result); - } -} -function HTTPError(message, providedCode, response, stack) { - const code = providedCode > 999 || providedCode < 100 ? -1 : providedCode; - const e = new Error(`${message}; code: ${code}`); - e.name = 'HTTPError'; - e.code = code; - if (stack) { - e.stack += `\nOriginal stack:\n${stack}`; - } - if (response) { - e.response = response; + return result.nodes; } - return e; } module.exports = LokiSnodeAPI; diff --git a/libloki/api.js b/libloki/api.js index e9c6dcc96..eab326d43 100644 --- a/libloki/api.js +++ b/libloki/api.js @@ -33,14 +33,6 @@ lokiAddressMessage, }); - // will be called once the transmission succeeded or failed - const callback = res => { - if (res.errors.length > 0) { - res.errors.forEach(error => log.error(error)); - } else { - log.info('Online broadcast message sent successfully'); - } - }; const options = { messageType: 'onlineBroadcast', isPing }; // Send a empty message with information about how to contact us directly const outgoingMessage = new textsecure.OutgoingMessage( @@ -49,7 +41,7 @@ [pubKey], // numbers content, // message true, // silent - callback, // callback + () => null, // callback options ); await outgoingMessage.sendToNumber(pubKey); diff --git a/libtextsecure/errors.js b/libtextsecure/errors.js index d6788957c..12aee4d25 100644 --- a/libtextsecure/errors.js +++ b/libtextsecure/errors.js @@ -127,18 +127,14 @@ } inherit(Error, UnregisteredUserError); - function EmptySwarmError(number, error) { + function EmptySwarmError(number, message) { // eslint-disable-next-line prefer-destructuring this.number = number.split('.')[0]; ReplayableError.call(this, { name: 'EmptySwarmError', - message: 'Could not get any swarm nodes to query', + message, }); - - if (error) { - appendStack(this, error); - } } inherit(ReplayableError, EmptySwarmError); From 0b6849c96a7a4fe97f3203edfeb14f342267ec1c Mon Sep 17 00:00:00 2001 From: Beaudan Date: Wed, 13 Mar 2019 16:41:13 +1100 Subject: [PATCH 2/2] Review comment plus added a todo --- js/modules/loki_snode_api.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/js/modules/loki_snode_api.js b/js/modules/loki_snode_api.js index e24be4426..0f83e7cc3 100644 --- a/js/modules/loki_snode_api.js +++ b/js/modules/loki_snode_api.js @@ -232,7 +232,9 @@ class LokiSnodeAPI { result = await response.text(); } - if (response.status !== 200 || !result.nodes || result.nodes === []) { + // TODO: Handle wrong swarm error from snode + + if (!response.ok || !result.nodes || result.nodes === []) { throw new window.textsecure.EmptySwarmError( pubKey, 'Could not retrieve swarm nodes'