Skip to content

Commit

Permalink
Merge pull request #97 from bugsnag/limit-stored-errors
Browse files Browse the repository at this point in the history
Limit number of stored errors
  • Loading branch information
kattrali committed Jan 20, 2016
2 parents 6b7fc3b + 712aa07 commit 7f0a626
Showing 1 changed file with 17 additions and 0 deletions.
17 changes: 17 additions & 0 deletions src/main/java/com/bugsnag/android/ErrorStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import java.io.File;
import java.io.FileWriter;
import java.io.Writer;
import java.util.Arrays;
import java.util.List;

import android.content.Context;

Expand All @@ -12,6 +14,7 @@
*/
class ErrorStore {
private static final String UNSENT_ERROR_PATH = "/bugsnag-errors/";
private static final int MAX_STORED_ERRORS = 100;

final Configuration config;
final String path;
Expand Down Expand Up @@ -77,6 +80,20 @@ public void run() {
void write(Error error) {
if(path == null) return;

// Limit number of saved errors to prevent disk space issues
File exceptionDir = new File(path);
if (exceptionDir.isDirectory()) {
File[] files = exceptionDir.listFiles();
if (files.length >= MAX_STORED_ERRORS) {
// Sort files then delete the first one (oldest timestamp)
Arrays.sort(files);

This comment has been minimized.

Copy link
@d4rken

d4rken Jan 20, 2016

Contributor

What guarantees that this always sorts such that index 0 is the oldest one?

Logger.warn(String.format("Discarding oldest error as stored error limit reached (%s)", files[0].getPath()));
if (!files[0].delete()) {
files[0].deleteOnExit();

This comment has been minimized.

Copy link
@d4rken

d4rken Jan 20, 2016

Contributor

Maybe we should check how many errors are above the LIMIT and delete multiple at once instead of only always the last.
Isn't it possible to log (and store) multiple errors that are either silent or don't crash the app (and VM)?
If delete fails we rely on deleteOnExit to work, but if that doesn't work as expected, we are above the 100 entry limit forever until the whole error store is emptied.
And deleteOnExit might not be very reliable. I'm also curious what is meant in the docs by "when the VM terminates normally".

Hm, the impact of this would be pretty minor though as eventually the error store should be emptied.

}
}
}

String filename = String.format("%s%d.json", path, System.currentTimeMillis());
Writer out = null;
try {
Expand Down

0 comments on commit 7f0a626

Please sign in to comment.