Fixes #231
Merged
Fixes #231
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kattrali
commented
Jan 24, 2018
| @@ -48,7 +48,7 @@ public int compare(File lhs, File rhs) { | |||
| void flushOnLaunch(final ErrorReportApiClient errorReportApiClient) { | |||
| final List<File> crashReports = findLaunchCrashReports(); | |||
|
|
|||
| if (crashReports.isEmpty() && config.getLaunchCrashThresholdMs() > 0) { | |||
| if (crashReports.isEmpty() && config.getLaunchCrashThresholdMs() == 0) { | |||
| flushAsync(errorReportApiClient); // if disabled or no startup crash, flush async | |||
There was a problem hiding this comment.
- if (crashReports.isEmpty() && config.getLaunchCrashThresholdMs() > 0) {
+ if (crashReports.isEmpty() && config.getLaunchCrashThresholdMs() == 0) {
// if disabled or no startup crash, flush asyncWhen launchCrashThresholdMs is greater than 0, any stored crashes which occurred before the threshold elapsed since launch would be sent immediately, correct? So launchCrashThresholdMs equal to 0 is disabling sending crashes on launch.
Breaking this one down,
- if (crashReports.isEmpty() && config.getLaunchCrashThresholdMs() == 0) {
+ boolean neverFlushOnLaunch = config.getLaunchCrashThresholdMs() == 0;
+ boolean noStartupCrashes = crashReports.isEmpty();
+ if (noStartupCrashes && neverFlushOnLaunch) {
// if disabled or no startup crash, flush asyncshouldn't this be an "or" instead?
- if (noStartupCrashes && neverFlushOnLaunch) {
+ if (neverFlushOnLaunch || noStartupCrashes) {
There was a problem hiding this comment.
This makes sense to me.
| writer.name("duration").value(getDuration()); | ||
| writer.name("durationInForeground").value(sessionTracker.getDurationInForeground(System.currentTimeMillis())); | ||
| writer.name("duration").value(getDurationMs()/1000); | ||
| writer.name("durationInForeground").value(sessionTracker.getDurationInForegroundMs(System.currentTimeMillis())/1000); |
There was a problem hiding this comment.
This is not introduced in this changeset, but these values are different if the session is delivered immediately vs retried after a long attempted network request.
| if (activityStarting) { | ||
| long noActivityRunningForMs = nowMs - activityLastStoppedAtMs.get(); | ||
|
|
||
| //TODO:SM Race condition between isEmpty and put |
There was a problem hiding this comment.
I didn't find a real-world case for this, even programmatically switching foreground activities rapidly.
startFirstSession tracks an automatic-style session if one has not yet been captured, recording the current activity. This avoids: * not reporting the current session because an activity is already active * starting a session manually which will result in duplication when a new activity is started
loopj
reviewed
Jan 26, 2018
| Async.run(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| //TODO:SM It would be good to optimise this |
| Async.run(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| //TODO:SM It would be good to optimise this |
| if (!storedFiles.isEmpty()) { | ||
| SessionTrackingPayload payload = new SessionTrackingPayload(storedFiles, client.appData); | ||
|
|
||
| //TODO:SM Reduce duplication here and above |
lemnik
pushed a commit
that referenced
this pull request
Jun 2, 2021
Rename properties on bugsnag extension that control request autoUpload
rich-bugsnag
pushed a commit
that referenced
this pull request
Sep 3, 2021
Update android build script
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reduces complexity of locking/synchronization, which reduces scope of ANRs