-
-
Notifications
You must be signed in to change notification settings - Fork 459
Avoid StrictMode warnings #4724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,34 +40,22 @@ public void processDirectory(final @NotNull File directory) { | |
try { | ||
logger.log(SentryLevel.DEBUG, "Processing dir. %s", directory.getAbsolutePath()); | ||
|
||
if (!directory.exists()) { | ||
final File[] filteredListFiles = directory.listFiles((d, name) -> isRelevantFileName(name)); | ||
if (filteredListFiles == null) { | ||
logger.log( | ||
SentryLevel.WARNING, | ||
"Directory '%s' doesn't exist. No cached events to send.", | ||
SentryLevel.ERROR, | ||
"Cache dir %s is null or is not a directory.", | ||
directory.getAbsolutePath()); | ||
return; | ||
} | ||
if (!directory.isDirectory()) { | ||
logger.log( | ||
SentryLevel.ERROR, "Cache dir %s is not a directory.", directory.getAbsolutePath()); | ||
return; | ||
} | ||
|
||
final File[] listFiles = directory.listFiles(); | ||
if (listFiles == null) { | ||
logger.log(SentryLevel.ERROR, "Cache dir %s is null.", directory.getAbsolutePath()); | ||
return; | ||
} | ||
|
||
final File[] filteredListFiles = directory.listFiles((d, name) -> isRelevantFileName(name)); | ||
|
||
logger.log( | ||
SentryLevel.DEBUG, | ||
"Processing %d items from cache dir %s", | ||
filteredListFiles != null ? filteredListFiles.length : 0, | ||
filteredListFiles.length, | ||
directory.getAbsolutePath()); | ||
|
||
for (File file : listFiles) { | ||
for (File file : filteredListFiles) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found we were filtering the files in the directory, but then looped over all the files anyway There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure that was an oversight. IMHO the risk is low as all non-envelope files will fail to be parsed / sent anyway. |
||
// it ignores .sentry-native database folder and new ones that might come up | ||
if (!file.isFile()) { | ||
logger.log(SentryLevel.DEBUG, "File %s is not a File.", file.getAbsolutePath()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm thinking this is too relaxing - e.g. the user init config (line 142) will also be under the LAX policy now, which is not what we want probably (e.g. if the user does I/O inside the
init { }
block this would not be flagged anymore). Can we be more specific and only wrap the actual offending calls into LAX?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was also thinking about resetting the policies before the config init (user callback) and then set it back to lax until the end of init()
Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that probably could do, but we're risking suppressing any new issues that might will have been introduced in the future outside the current ones. I'd really prefer to only target specific issues and set LAX as tight as possible, if it's not a big lift.