Limit number of stored errors #97
Conversation
|
Looks good. This makes sense, though I think in the case that we reach the limit, we should probably discard the oldest errors instead of the newest ones. I'm not sure on what is a reasonable limit though. |
|
Changed to delete the oldest error and store the new one. 100 seems like a reasonable number - if you have that many errors it is most likely the same thing repeatedly so there is probably limited value to reporting additional instances anyway. |
|
Awesome! 👍 |
|
How much space do 100 errors take up? Where are they stored by default? |
|
@d4rken The files are stored in the cache directory. |
|
The size will depend on lots of things like how big/deep the stack traces are and what custom meta-data is being added, but each error could end up being 10s of KB each at least. Storing 100 is not going to take up a huge amount of space but seems sensible to be conservative about the number of errors to store. Happy to change it if anyone has a different opinion though! |
|
Seems reasonable as default number. |
Limit number of stored errors
|
I think code comments don't show anymore when the PR is merged? Are my concerns valid or did i miss something? I must admit I'm not 100% versed in how the offline storage works at the moment. 7f0a626#commitcomment-15562085 7f0a626#commitcomment-15562415 If these are valid i would make an issue ticket and when I find time a PR. |
|
@d4rken I thought I'd reply here for visibility:
It ultimately uses the default sort from File.compareTo() which should order alphabetically. As the files are named with a timestamp, the smallest value (oldest time) is always at index 0.
I considered that we may end up with more than 100 if delete fails, but as it's an arbitrary limit I don't think it matters too much (as you suggest). |
|
Ok 👍 |
Android Changelog ==== ## 3.4.0 (2016-03-09) ### Enhancements - Limit the number of stored errors [Duncan Hewett](https://github.com/duncanhewett) [#97](bugsnag/bugsnag-android#97) ### Bug Fixes - Fix `ConcurrentModificationException` which could occur when saving breadcrumbs [Duncan Hewett](https://github.com/duncanhewett) [#98](bugsnag/bugsnag-android#98) - Localize all numbers in error metrics [Delisa Mason](https://github.com/kattrali) [#100](bugsnag/bugsnag-android#100) 3.3.0 (2016-01-18) ----- ### Enhancements - Change distribution method to be .aar only [Lars Grefer](https://github.com/larsgrefer) [#91](bugsnag/bugsnag-android#91) - Skip sending empty device data values [Matthias Urhahn](https://github.com/d4rken) [#96](bugsnag/bugsnag-android#96) - Remove the need for synthetic methods [Jake Wharton](https://github.com/JakeWharton) [#87](bugsnag/bugsnag-android#87) 3.2.7 (2015-12-10) ----- ### Enhancements - Add additional check to ensure the cache of uploaded errors are deleted [#80](bugsnag/bugsnag-android#80) ### Bug Fixes - Fix exception which occurs when `appContext.getResources()` is null [#78](bugsnag/bugsnag-android#78) - Fix bug preventing `maxBreadcrumbs` from being set [David Wu](https://github.com/wuman) [#70](bugsnag/bugsnag-android#70) 3.2.6 ----- - Add blocking API - Fix NPE issue - Concurrent adding to tabs - Thread Safe DateUtils#toISO8601
Limit the number of errors stored for later sending to prevent possible disk space issues (see #23).
Stores up to 100 errors and discards any subsequent errors. First 100 errors are stored on the basis that they are likely to be the most interesting. Is this sensible?