Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

fix: converting UTC and ISO timestamp when missing Locale/TimeZone do not error #505

Merged
merged 2 commits into from
Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ final class DefaultAndroidEventProcessor implements EventProcessor {
@TestOnly static final String EMULATOR = "emulator";

// it could also be a parameter and get from Sentry.init(...)
private static final Date appStartTime = DateUtils.getCurrentDateTime();
private static final @Nullable Date appStartTime = DateUtils.getCurrentDateTimeOrNull();

@TestOnly final Context context;

Expand Down Expand Up @@ -397,10 +397,15 @@ private TimeZone getTimeZone() {
}

@SuppressWarnings("JdkObsolete")
private @NotNull Date getBootTime() {
// if user changes time, will give a wrong answer, consider ACTION_TIME_CHANGED
return DateUtils.getDateTime(
new Date(System.currentTimeMillis() - SystemClock.elapsedRealtime()));
private @Nullable Date getBootTime() {
try {
// if user changes time, will give a wrong answer, consider ACTION_TIME_CHANGED
return DateUtils.getDateTime(
new Date(System.currentTimeMillis() - SystemClock.elapsedRealtime()));
} catch (IllegalArgumentException e) {
logger.log(SentryLevel.ERROR, e, "Error getting the device's boot time.");
}
return null;
}

private @NotNull String getResolution(final @NotNull DisplayMetrics displayMetrics) {
Expand Down
6 changes: 3 additions & 3 deletions sentry-core/src/main/java/io/sentry/core/Breadcrumb.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
public final class Breadcrumb implements Cloneable, IUnknownPropertiesConsumer {

/** A timestamp representing when the breadcrumb occurred. */
private final @NotNull Date timestamp;
private final @Nullable Date timestamp;

/** If a message is provided, its rendered as text and the whitespace is preserved. */
private @Nullable String message;
Expand All @@ -39,13 +39,13 @@ public final class Breadcrumb implements Cloneable, IUnknownPropertiesConsumer {
*
* @param timestamp the timestamp
*/
Breadcrumb(final @NotNull Date timestamp) {
Breadcrumb(final @Nullable Date timestamp) {
this.timestamp = timestamp;
}

/** Breadcrumb ctor */
public Breadcrumb() {
this(DateUtils.getCurrentDateTime());
this(DateUtils.getCurrentDateTimeOrNull());
}

/**
Expand Down
28 changes: 22 additions & 6 deletions sentry-core/src/main/java/io/sentry/core/DateUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.TimeZone;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

/** Utilities to deal with dates */
@ApiStatus.Internal
Expand All @@ -26,7 +27,7 @@ private DateUtils() {}
*/
public static @NotNull String getTimestampIsoFormat(final @NotNull Date date) {
final TimeZone tz = TimeZone.getTimeZone(UTC);
final DateFormat df = new SimpleDateFormat(ISO_FORMAT_WITH_MILLIS, Locale.US);
final DateFormat df = new SimpleDateFormat(ISO_FORMAT_WITH_MILLIS, Locale.ROOT);
df.setTimeZone(tz);
return df.format(date);
}
Expand All @@ -37,11 +38,25 @@ private DateUtils() {}
* @return the ISO UTC date and time
*/
@SuppressWarnings("JdkObsolete")
public static @NotNull Date getCurrentDateTime() {
public static @NotNull Date getCurrentDateTime() throws IllegalArgumentException {
final String timestampIsoFormat = getTimestampIsoFormat(new Date());
return getDateTime(timestampIsoFormat);
}

/**
* Get the current date and time as ISO UTC or null if not available
*
* @return the ISO UTC date and time or null
*/
public static @Nullable Date getCurrentDateTimeOrNull() throws IllegalArgumentException {
try {
return getCurrentDateTime();
} catch (IllegalArgumentException ignored) {
// error getting current device's timestamp due to eg locale problems
}
return null;
}

/**
* Get Java Date from UTC timestamp format
*
Expand All @@ -51,11 +66,11 @@ private DateUtils() {}
public static @NotNull Date getDateTime(final @NotNull String timestamp)
throws IllegalArgumentException {
try {
return new SimpleDateFormat(ISO_FORMAT_WITH_MILLIS, Locale.US).parse(timestamp);
return new SimpleDateFormat(ISO_FORMAT_WITH_MILLIS, Locale.ROOT).parse(timestamp);
} catch (ParseException e) {
try {
// to keep compatibility with older envelopes
return new SimpleDateFormat(ISO_FORMAT, Locale.US).parse(timestamp);
return new SimpleDateFormat(ISO_FORMAT, Locale.ROOT).parse(timestamp);
} catch (ParseException ignored) {
// invalid timestamp format
}
Expand Down Expand Up @@ -90,7 +105,7 @@ private DateUtils() {}
* @return the ISO formatted date with millis precision.
*/
public static @NotNull String getTimestamp(final @NotNull Date date) {
final DateFormat df = new SimpleDateFormat(ISO_FORMAT_WITH_MILLIS, Locale.US);
final DateFormat df = new SimpleDateFormat(ISO_FORMAT_WITH_MILLIS, Locale.ROOT);
return df.format(date);
}

Expand All @@ -100,7 +115,8 @@ private DateUtils() {}
* @param date the Date with local timezone
* @return the Date UTC timezone
*/
public static @NotNull Date getDateTime(final @NotNull Date date) {
public static @NotNull Date getDateTime(final @NotNull Date date)
throws IllegalArgumentException {
final String timestampIsoFormat = getTimestampIsoFormat(date);
return getDateTime(timestampIsoFormat);
}
Expand Down
2 changes: 1 addition & 1 deletion sentry-core/src/main/java/io/sentry/core/SentryEvent.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public SentryEvent(Throwable throwable) {
}

public SentryEvent() {
this(new SentryId(), DateUtils.getCurrentDateTime());
this(new SentryId(), DateUtils.getCurrentDateTimeOrNull());
}

@TestOnly
Expand Down
20 changes: 12 additions & 8 deletions sentry-core/src/main/java/io/sentry/core/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ public Session(
final @NotNull String release) {
this(
State.Ok,
DateUtils.getCurrentDateTime(),
DateUtils.getCurrentDateTime(),
DateUtils.getCurrentDateTimeOrNull(),
DateUtils.getCurrentDateTimeOrNull(),
0,
distinctId,
UUID.randomUUID(),
Expand Down Expand Up @@ -166,7 +166,7 @@ public int errorCount() {

/** Ends a session and update its values */
public void end() {
end(DateUtils.getCurrentDateTime());
end(DateUtils.getCurrentDateTimeOrNull());
}

/**
Expand All @@ -186,11 +186,13 @@ public void end(final @Nullable Date timestamp) {
if (timestamp != null) {
this.timestamp = timestamp;
} else {
this.timestamp = DateUtils.getCurrentDateTime();
this.timestamp = DateUtils.getCurrentDateTimeOrNull();
}

duration = calculateDurationTime(this.timestamp);
sequence = getSequenceTimestamp(this.timestamp);
if (this.timestamp != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely worth not having sessions enabled at all for this device since they'll be broken

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, SessionAdapter will drop on deserialization at least.

duration = calculateDurationTime(this.timestamp);
sequence = getSequenceTimestamp(this.timestamp);
}
}
}

Expand Down Expand Up @@ -234,8 +236,10 @@ public boolean update(

if (sessionHasBeenUpdated) {
init = null;
timestamp = DateUtils.getCurrentDateTime();
sequence = getSequenceTimestamp(timestamp);
timestamp = DateUtils.getCurrentDateTimeOrNull();
if (timestamp != null) {
sequence = getSequenceTimestamp(timestamp);
}
}
return sessionHasBeenUpdated;
}
Expand Down
15 changes: 13 additions & 2 deletions sentry-core/src/main/java/io/sentry/core/SessionAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.UUID;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Internal
public final class SessionAdapter extends TypeAdapter<Session> {
Expand Down Expand Up @@ -133,7 +134,7 @@ public Session read(JsonReader reader) throws IOException {
init = reader.nextBoolean();
break;
case "started":
started = DateUtils.getDateTime(reader.nextString());
started = converTimeStamp(reader.nextString(), "started");
break;
case "status":
status = Session.State.valueOf(StringUtils.capitalize(reader.nextString()));
Expand All @@ -148,7 +149,7 @@ public Session read(JsonReader reader) throws IOException {
duration = reader.nextDouble();
break;
case "timestamp":
timestamp = DateUtils.getDateTime(reader.nextString());
timestamp = converTimeStamp(reader.nextString(), "timestamp");
break;
case "attrs":
{
Expand Down Expand Up @@ -203,4 +204,14 @@ public Session read(JsonReader reader) throws IOException {
environment,
release);
}

private @Nullable Date converTimeStamp(
final @NotNull String timestamp, final @NotNull String field) {
try {
return DateUtils.getDateTime(timestamp);
} catch (IllegalArgumentException e) {
logger.log(SentryLevel.ERROR, e, "Error converting session (%s) field.", field);
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ private Date getTimestampFromCrashMarkerFile(final @NotNull File markerFile) {
return DateUtils.getDateTime(timestamp);
} catch (IOException e) {
options.getLogger().log(ERROR, "Error reading the crash marker file.", e);
} catch (IllegalArgumentException e) {
options.getLogger().log(SentryLevel.ERROR, e, "Error converting the crash timestamp.");
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ public static void main(String[] args) {
// your Sentry project/dashboard
options.setDsn("https://[email protected]/1808954");

// All events get assigned to the release. See more at https://docs.sentry.io/workflow/releases/
// All events get assigned to the release. See more at
// https://docs.sentry.io/workflow/releases/
options.setRelease("[email protected]+1");

// Modifications to event before it goes out. Could replace the event altogether
Expand Down Expand Up @@ -50,9 +51,13 @@ public static void main(String[] args) {
options.setDebug(true);
// To change the verbosity, use:
// By default it's DEBUG.
options.setDiagnosticLevel(SentryLevel.ERROR); // A good option to have SDK debug log in prod is to use only level ERROR here.
options.setDiagnosticLevel(
SentryLevel
.ERROR); // A good option to have SDK debug log in prod is to use only level
// ERROR here.

// Exclude frames from some packages from being "inApp" so are hidden by default in Sentry UI:
// Exclude frames from some packages from being "inApp" so are hidden by default in Sentry
// UI:
options.addInAppExclude("org.jboss");
});

Expand Down Expand Up @@ -110,7 +115,8 @@ public static void main(String[] args) {
Sentry.captureEvent(event, SentryLevel.DEBUG);
}

// All events that have not been sent yet are being flushed on JVM exit. Events can be also flushed manually:
// All events that have not been sent yet are being flushed on JVM exit. Events can be also
// flushed manually:
// Sentry.close();
}

Expand Down