From 6e4b06fcb0cc0c074d2266074a60e01a317541dd Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Thu, 13 Jul 2023 16:37:44 +0100 Subject: [PATCH 1/4] refactor: error logging on connectivity check Replaces the stacktrace dump with a debug log. These errors are part of the connectivity check, so may not warrant a full error log. --- lib/src/main/java/io/ably/lib/http/HttpCore.java | 1 - lib/src/main/java/io/ably/lib/transport/ConnectionManager.java | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/main/java/io/ably/lib/http/HttpCore.java b/lib/src/main/java/io/ably/lib/http/HttpCore.java index 035848475..e052043d8 100644 --- a/lib/src/main/java/io/ably/lib/http/HttpCore.java +++ b/lib/src/main/java/io/ably/lib/http/HttpCore.java @@ -251,7 +251,6 @@ T httpExecute(HttpURLConnection conn, String method, Param[] headers, Reques rawHttpListener.onRawHttpResponse(id, method, response); } } catch(IOException ioe) { - ioe.printStackTrace(); if(rawHttpListener != null) { rawHttpListener.onRawHttpException(id, method, ioe); } diff --git a/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java b/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java index dfb3d8fe9..392e3a4ce 100644 --- a/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java +++ b/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java @@ -1591,6 +1591,7 @@ protected boolean checkConnectivity() { try { return HttpHelpers.getUrlString(ably.httpCore, INTERNET_CHECK_URL).contains(INTERNET_CHECK_OK); } catch(AblyException e) { + Log.d(TAG, "Exception whilst checking connectivity", e); return false; } } From 5a47ac1dcb613d196a854cfe70e1b0177eec27af Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Thu, 13 Jul 2023 16:55:48 +0100 Subject: [PATCH 2/4] refactor: replace message serialization stack traces with normal error logs Rather than logging the stack trace to the system error log, do a standard Log.e so it can be received by custom error handlers. --- lib/src/main/java/io/ably/lib/types/Message.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/types/Message.java b/lib/src/main/java/io/ably/lib/types/Message.java index 8b416a4d0..9551c0c26 100644 --- a/lib/src/main/java/io/ably/lib/types/Message.java +++ b/lib/src/main/java/io/ably/lib/types/Message.java @@ -274,7 +274,7 @@ public static Message[] fromEncodedArray(JsonArray messageArray, ChannelOptions } return messages; } catch(Exception e) { - e.printStackTrace(); + Log.e(Message.class.getName(), e.getMessage(), e); throw MessageDecodeException.fromDescription(e.getMessage()); } } @@ -295,7 +295,7 @@ public static Message[] fromEncodedArray(String messagesArray, ChannelOptions ch JsonArray jsonArray = Serialisation.gson.fromJson(messagesArray, JsonArray.class); return fromEncodedArray(jsonArray, channelOptions); } catch(Exception e) { - e.printStackTrace(); + Log.e(Message.class.getName(), e.getMessage(), e); throw MessageDecodeException.fromDescription(e.getMessage()); } } @@ -341,7 +341,7 @@ public Message deserialize(JsonElement json, Type typeOfT, JsonDeserializationCo try { message.read((JsonObject)json); } catch (MessageDecodeException e) { - e.printStackTrace(); + Log.e(Message.class.getName(), e.getMessage(), e); throw new JsonParseException("Failed to deserialize Message from JSON.", e); } return message; From 5176e80af769a04fcb481b1217c99beac3fa5db7 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Thu, 13 Jul 2023 20:13:14 +0100 Subject: [PATCH 3/4] add comments to illustrate verbose logging --- lib/src/main/java/io/ably/lib/http/HttpCore.java | 3 +++ lib/src/main/java/io/ably/lib/transport/ConnectionManager.java | 1 + .../main/java/io/ably/lib/transport/WebSocketTransport.java | 3 +++ 3 files changed, 7 insertions(+) diff --git a/lib/src/main/java/io/ably/lib/http/HttpCore.java b/lib/src/main/java/io/ably/lib/http/HttpCore.java index e052043d8..58154040f 100644 --- a/lib/src/main/java/io/ably/lib/http/HttpCore.java +++ b/lib/src/main/java/io/ably/lib/http/HttpCore.java @@ -216,12 +216,14 @@ T httpExecute(HttpURLConnection conn, String method, Param[] headers, Reques byte[] body = null; if(requestBody != null) { body = prepareRequestBody(requestBody, conn); + // Logging level is checked before logging for performance reasons in building the entry if (Log.level <= Log.VERBOSE) Log.v(TAG, System.lineSeparator() + new String(body)); } /* log raw request details */ Map> requestProperties = conn.getRequestProperties(); + // Logging level is checked before logging for performance reasons in building the entry if (Log.level <= Log.VERBOSE) { Log.v(TAG, "HTTP request: " + conn.getURL() + " " + method); if (credentialsIncluded) @@ -399,6 +401,7 @@ private Response readResponse(HttpURLConnection connection) throws IOException { for (Map.Entry> entry : caseSensitiveHeaders.entrySet()) { if (entry.getKey() != null) { response.headers.put(entry.getKey().toLowerCase(Locale.ROOT), entry.getValue()); + // Logging level is checked before logging for performance reasons in building the entry if (Log.level <= Log.VERBOSE) for (String val : entry.getValue()) Log.v(TAG, entry.getKey() + ": " + val); diff --git a/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java b/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java index 392e3a4ce..7daba7758 100644 --- a/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java +++ b/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java @@ -1136,6 +1136,7 @@ public void onMessage(ITransport transport, ProtocolMessage message) throws Ably if (transport != null && this.transport != transport) { return; } + // Logging level is checked before logging for performance reasons in building the entry if (Log.level <= Log.VERBOSE) { Log.v(TAG, "onMessage() (transport = " + transport + "): " + message.action + ": " + new String(ProtocolSerializer.writeJSON(message))); } diff --git a/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java b/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java index 62a1d4b93..14a35983f 100644 --- a/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java +++ b/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java @@ -110,12 +110,15 @@ public void send(ProtocolMessage msg) throws AblyException { try { if(channelBinaryMode) { byte[] encodedMsg = ProtocolSerializer.writeMsgpack(msg); + + // Logging level is checked before logging for performance reasons in building the entry if (Log.level <= Log.VERBOSE) { ProtocolMessage decodedMsg = ProtocolSerializer.readMsgpack(encodedMsg); Log.v(TAG, "send(): " + decodedMsg.action + ": " + new String(ProtocolSerializer.writeJSON(decodedMsg))); } wsConnection.send(encodedMsg); } else { + // Logging level is checked before logging for performance reasons in building the entry if (Log.level <= Log.VERBOSE) Log.v(TAG, "send(): " + new String(ProtocolSerializer.writeJSON(msg))); wsConnection.send(ProtocolSerializer.writeJSON(msg)); From 25da9a9c1b2c923b930f4e238886171df912130e Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Thu, 13 Jul 2023 20:31:32 +0100 Subject: [PATCH 4/4] reword comment --- lib/src/main/java/io/ably/lib/http/HttpCore.java | 6 +++--- .../main/java/io/ably/lib/transport/ConnectionManager.java | 2 +- .../main/java/io/ably/lib/transport/WebSocketTransport.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/http/HttpCore.java b/lib/src/main/java/io/ably/lib/http/HttpCore.java index 58154040f..8277fe3d9 100644 --- a/lib/src/main/java/io/ably/lib/http/HttpCore.java +++ b/lib/src/main/java/io/ably/lib/http/HttpCore.java @@ -216,14 +216,14 @@ T httpExecute(HttpURLConnection conn, String method, Param[] headers, Reques byte[] body = null; if(requestBody != null) { body = prepareRequestBody(requestBody, conn); - // Logging level is checked before logging for performance reasons in building the entry + // Check the logging level to avoid performance hit associated with building the message if (Log.level <= Log.VERBOSE) Log.v(TAG, System.lineSeparator() + new String(body)); } /* log raw request details */ Map> requestProperties = conn.getRequestProperties(); - // Logging level is checked before logging for performance reasons in building the entry + // Check the logging level to avoid performance hit associated with building the message if (Log.level <= Log.VERBOSE) { Log.v(TAG, "HTTP request: " + conn.getURL() + " " + method); if (credentialsIncluded) @@ -401,7 +401,7 @@ private Response readResponse(HttpURLConnection connection) throws IOException { for (Map.Entry> entry : caseSensitiveHeaders.entrySet()) { if (entry.getKey() != null) { response.headers.put(entry.getKey().toLowerCase(Locale.ROOT), entry.getValue()); - // Logging level is checked before logging for performance reasons in building the entry + // Check the logging level to avoid performance hit associated with building the message if (Log.level <= Log.VERBOSE) for (String val : entry.getValue()) Log.v(TAG, entry.getKey() + ": " + val); diff --git a/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java b/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java index 7daba7758..29f33706a 100644 --- a/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java +++ b/lib/src/main/java/io/ably/lib/transport/ConnectionManager.java @@ -1136,7 +1136,7 @@ public void onMessage(ITransport transport, ProtocolMessage message) throws Ably if (transport != null && this.transport != transport) { return; } - // Logging level is checked before logging for performance reasons in building the entry + // Check the logging level to avoid performance hit associated with building the message if (Log.level <= Log.VERBOSE) { Log.v(TAG, "onMessage() (transport = " + transport + "): " + message.action + ": " + new String(ProtocolSerializer.writeJSON(message))); } diff --git a/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java b/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java index 14a35983f..ba3399b7e 100644 --- a/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java +++ b/lib/src/main/java/io/ably/lib/transport/WebSocketTransport.java @@ -111,14 +111,14 @@ public void send(ProtocolMessage msg) throws AblyException { if(channelBinaryMode) { byte[] encodedMsg = ProtocolSerializer.writeMsgpack(msg); - // Logging level is checked before logging for performance reasons in building the entry + // Check the logging level to avoid performance hit associated with building the message if (Log.level <= Log.VERBOSE) { ProtocolMessage decodedMsg = ProtocolSerializer.readMsgpack(encodedMsg); Log.v(TAG, "send(): " + decodedMsg.action + ": " + new String(ProtocolSerializer.writeJSON(decodedMsg))); } wsConnection.send(encodedMsg); } else { - // Logging level is checked before logging for performance reasons in building the entry + // Check the logging level to avoid performance hit associated with building the message if (Log.level <= Log.VERBOSE) Log.v(TAG, "send(): " + new String(ProtocolSerializer.writeJSON(msg))); wsConnection.send(ProtocolSerializer.writeJSON(msg));