Skip to content

Commit 192fe5d

Browse files
committed
Refine logging to be prod ready
1 parent b248df7 commit 192fe5d

File tree

3 files changed

+19
-38
lines changed

3 files changed

+19
-38
lines changed

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ public void testWriteUserData_noFields() throws Exception {
118118
}
119119

120120
@Test
121+
// TODO(b/375437048): Let @daymxn know if this test fails. It's flaky and running away from me:(
121122
public void testWriteUserData_singleField() throws Exception {
122123
crashlyticsWorkers.diskWrite.submit(
123124
() -> {
@@ -167,6 +168,7 @@ public void testWriteUserData_unicode() throws Exception {
167168
}
168169

169170
@Test
171+
// TODO(b/375437048): Let @daymxn know if this test fails. It's flaky and running away from me:(
170172
public void testWriteUserData_escaped() throws Exception {
171173
crashlyticsWorkers.diskWrite.submit(
172174
() -> {

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStore.java

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public String readUserId(String sessionId) {
7373
final File f = getUserDataFileForSession(sessionId);
7474
if (!f.exists() || f.length() == 0) {
7575
Logger.getLogger().d("No userId set for session " + sessionId);
76-
safeDeleteCorruptFile(f, "No userId set for session " + sessionId);
76+
safeDeleteCorruptFile(f);
7777
return null;
7878
}
7979

@@ -85,7 +85,7 @@ public String readUserId(String sessionId) {
8585
return userId;
8686
} catch (Exception e) {
8787
Logger.getLogger().w("Error deserializing user metadata.", e);
88-
safeDeleteCorruptFile(f, "Error deserializing user metadata: " + e);
88+
safeDeleteCorruptFile(f);
8989
} finally {
9090
CommonUtils.closeOrLog(is, "Failed to close user metadata file.");
9191
}
@@ -107,7 +107,7 @@ public void writeKeyData(String sessionId, Map<String, String> keyData, boolean
107107
writer.flush();
108108
} catch (Exception e) {
109109
Logger.getLogger().w("Error serializing key/value metadata.", e);
110-
safeDeleteCorruptFile(f, "Error serializing key/value metadata." + e);
110+
safeDeleteCorruptFile(f);
111111
} finally {
112112
CommonUtils.closeOrLog(writer, "Failed to close key/value metadata file.");
113113
}
@@ -121,19 +121,7 @@ Map<String, String> readKeyData(String sessionId, boolean isInternal) {
121121
final File f =
122122
isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
123123
if (!f.exists() || f.length() == 0) {
124-
if (!f.exists()) {
125-
safeDeleteCorruptFile(
126-
f,
127-
String.format(
128-
"The file at path \"%s\" doesn't exists for session \"%s\"",
129-
f.getAbsolutePath(), sessionId));
130-
} else {
131-
safeDeleteCorruptFile(
132-
f,
133-
String.format(
134-
"The file at path \"%s\" has a length of zero for session \"%s\"",
135-
f.getAbsolutePath(), sessionId));
136-
}
124+
safeDeleteCorruptFile(f, "The file has a length of zero for session: " + sessionId);
137125
return Collections.emptyMap();
138126
}
139127

@@ -143,7 +131,7 @@ Map<String, String> readKeyData(String sessionId, boolean isInternal) {
143131
return jsonToKeysData(CommonUtils.streamToString(is));
144132
} catch (Exception e) {
145133
Logger.getLogger().w("Error deserializing user metadata.", e);
146-
safeDeleteCorruptFile(f, "Error deserializing user metadata." + e);
134+
safeDeleteCorruptFile(f);
147135
} finally {
148136
CommonUtils.closeOrLog(is, "Failed to close user metadata file.");
149137
}
@@ -153,20 +141,7 @@ Map<String, String> readKeyData(String sessionId, boolean isInternal) {
153141
public List<RolloutAssignment> readRolloutsState(String sessionId) {
154142
final File f = getRolloutsStateForSession(sessionId);
155143
if (!f.exists() || f.length() == 0) {
156-
if (!f.exists()) {
157-
safeDeleteCorruptFile(
158-
f,
159-
String.format(
160-
"The file at path \"%s\" doesn't exists for session \"%s\"",
161-
f.getAbsolutePath(), sessionId));
162-
} else {
163-
safeDeleteCorruptFile(
164-
f,
165-
String.format(
166-
"The file at path \"%s\" has a length of zero for session \"%s\"",
167-
f.getAbsolutePath(), sessionId));
168-
}
169-
144+
safeDeleteCorruptFile(f, "The file has a length of zero for session: " + sessionId);
170145
return Collections.emptyList();
171146
}
172147

@@ -179,7 +154,7 @@ public List<RolloutAssignment> readRolloutsState(String sessionId) {
179154
return rolloutsState;
180155
} catch (Exception e) {
181156
Logger.getLogger().w("Error deserializing rollouts state.", e);
182-
safeDeleteCorruptFile(f, "Error deserializing rollouts state." + e);
157+
safeDeleteCorruptFile(f);
183158
} finally {
184159
CommonUtils.closeOrLog(is, "Failed to close rollouts state file.");
185160
}
@@ -201,7 +176,7 @@ public void writeRolloutState(String sessionId, List<RolloutAssignment> rollouts
201176
writer.flush();
202177
} catch (Exception e) {
203178
Logger.getLogger().w("Error serializing rollouts state.", e);
204-
safeDeleteCorruptFile(f, "Error serializing rollouts state." + e);
179+
safeDeleteCorruptFile(f);
205180
} finally {
206181
CommonUtils.closeOrLog(writer, "Failed to close rollouts state file.");
207182
}
@@ -295,13 +270,17 @@ private static String valueOrNull(JSONObject json, String key) {
295270
return !json.isNull(key) ? json.optString(key, null) : null;
296271
}
297272

298-
private static void safeDeleteCorruptFile(File file, String reason) {
273+
private static void safeDeleteCorruptFile(File file) {
299274
if (file.exists() && file.delete()) {
275+
Logger.getLogger().i("Deleted corrupt file: " + file.getAbsolutePath());
276+
}
277+
}
300278

279+
// TODO(b/375437048): Remove when fixed
280+
private static void safeDeleteCorruptFile(File file, String reason) {
281+
if (file.exists() && file.delete()) {
301282
Logger.getLogger()
302-
.i(
303-
String.format(
304-
"Deleted corrupt file due to \"%s\": %s", reason, file.getAbsolutePath()));
283+
.i(String.format("Deleted corrupt file: %s\nReason: %s", file.getAbsolutePath(), reason));
305284
}
306285
}
307286
}

firebase-messaging/firebase-messaging.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ dependencies {
9595
api('com.google.firebase:firebase-datatransport:18.2.0') {
9696
exclude group: 'com.google.firebase', module: 'firebase-common'
9797
exclude group: 'com.google.firebase', module: 'firebase-components'
98-
}
98+
}
9999
api 'com.google.firebase:firebase-encoders:17.0.0'
100100
api 'com.google.firebase:firebase-encoders-json:18.0.0'
101101
api "com.google.firebase:firebase-encoders-proto:16.0.0"

0 commit comments

Comments
 (0)