Skip to content

Commit 2989106

Browse files
romtsnromtsn
authored
Fix json parsing of nullable/empty fields (getsentry#2968)
1 parent 7e47940 commit 2989106

19 files changed

Lines changed: 114 additions & 45 deletions

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@
1818
- [changelog](https://github.com/getsentry/sentry-native/blob/master/CHANGELOG.md#066)
1919
- [diff](https://github.com/getsentry/sentry-native/compare/0.6.5...0.6.6)
2020

21+
### Fixes
22+
23+
- Fix json parsing of nullable/empty fields for Hybrid SDKs ([#2968](https://github.com/getsentry/sentry-java/pull/2968))
24+
- (Internal) Rename `nextList` to `nextListOrNull` to actually match what the method does
25+
- (Hybrid) Check if there's any object in a collection before trying to parse it (which prevents the "Failed to deserilize object in list" log message)
26+
- (Hybrid) If a date can't be parsed as an ISO timestamp, attempts to parse it as millis silently, without printing a log message
27+
- (Hybrid) If `op` is not defined as part of `SpanContext`, fallback to an empty string, because the filed is optional in the spec
28+
2129
## 6.30.0
2230

2331
### Features

‎sentry/api/sentry.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ public final class io/sentry/JsonObjectReader : io/sentry/vendor/gson/stream/Jso
770770
public fun nextFloat ()Ljava/lang/Float;
771771
public fun nextFloatOrNull ()Ljava/lang/Float;
772772
public fun nextIntegerOrNull ()Ljava/lang/Integer;
773-
public fun nextList (Lio/sentry/ILogger;Lio/sentry/JsonDeserializer;)Ljava/util/List;
773+
public fun nextListOrNull (Lio/sentry/ILogger;Lio/sentry/JsonDeserializer;)Ljava/util/List;
774774
public fun nextLongOrNull ()Ljava/lang/Long;
775775
public fun nextMapOrNull (Lio/sentry/ILogger;Lio/sentry/JsonDeserializer;)Ljava/util/Map;
776776
public fun nextObjectOrNull ()Ljava/lang/Object;

‎sentry/src/main/java/io/sentry/JsonObjectReader.java‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -81,21 +81,23 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
8181
}
8282
}
8383

84-
public <T> @Nullable List<T> nextList(
84+
public <T> @Nullable List<T> nextListOrNull(
8585
@NotNull ILogger logger, @NotNull JsonDeserializer<T> deserializer) throws IOException {
8686
if (peek() == JsonToken.NULL) {
8787
nextNull();
8888
return null;
8989
}
9090
beginArray();
9191
List<T> list = new ArrayList<>();
92-
do {
93-
try {
94-
list.add(deserializer.deserialize(this, logger));
95-
} catch (Exception e) {
96-
logger.log(SentryLevel.ERROR, "Failed to deserialize object in list.", e);
97-
}
98-
} while (peek() == JsonToken.BEGIN_OBJECT);
92+
if (hasNext()) {
93+
do {
94+
try {
95+
list.add(deserializer.deserialize(this, logger));
96+
} catch (Exception e) {
97+
logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e);
98+
}
99+
} while (peek() == JsonToken.BEGIN_OBJECT);
100+
}
99101
endArray();
100102
return list;
101103
}
@@ -108,14 +110,16 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
108110
}
109111
beginObject();
110112
Map<String, T> map = new HashMap<>();
111-
do {
112-
try {
113-
String key = nextName();
114-
map.put(key, deserializer.deserialize(this, logger));
115-
} catch (Exception e) {
116-
logger.log(SentryLevel.ERROR, "Failed to deserialize object in map.", e);
117-
}
118-
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
113+
if (hasNext()) {
114+
do {
115+
try {
116+
String key = nextName();
117+
map.put(key, deserializer.deserialize(this, logger));
118+
} catch (Exception e) {
119+
logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e);
120+
}
121+
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
122+
}
119123

120124
endObject();
121125
return map;
@@ -144,16 +148,12 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
144148
}
145149
try {
146150
return DateUtils.getDateTime(dateString);
147-
} catch (Exception e) {
148-
logger.log(
149-
SentryLevel.DEBUG,
150-
"Error when deserializing UTC timestamp format, it might be millis timestamp format.",
151-
e);
152-
}
153-
try {
154-
return DateUtils.getDateTimeWithMillisPrecision(dateString);
155-
} catch (Exception e) {
156-
logger.log(SentryLevel.ERROR, "Error when deserializing millis timestamp format.", e);
151+
} catch (Exception ignored) {
152+
try {
153+
return DateUtils.getDateTimeWithMillisPrecision(dateString);
154+
} catch (Exception e) {
155+
logger.log(SentryLevel.ERROR, "Error when deserializing millis timestamp format.", e);
156+
}
157157
}
158158
return null;
159159
}

‎sentry/src/main/java/io/sentry/JsonSerializer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public JsonSerializer(@NotNull SentryOptions options) {
134134
return (T) jsonObjectReader.nextObjectOrNull();
135135
}
136136

137-
return (T) jsonObjectReader.nextList(options.getLogger(), elementDeserializer);
137+
return (T) jsonObjectReader.nextListOrNull(options.getLogger(), elementDeserializer);
138138
} else {
139139
return (T) jsonObjectReader.nextObjectOrNull();
140140
}

‎sentry/src/main/java/io/sentry/ProfilingTraceData.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ public static final class Deserializer implements JsonDeserializer<ProfilingTrac
556556
break;
557557
case JsonKeys.TRANSACTION_LIST:
558558
List<ProfilingTransactionData> transactions =
559-
reader.nextList(logger, new ProfilingTransactionData.Deserializer());
559+
reader.nextListOrNull(logger, new ProfilingTransactionData.Deserializer());
560560
if (transactions != null) {
561561
data.transactions.addAll(transactions);
562562
}

‎sentry/src/main/java/io/sentry/SentryBaseEvent.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ public boolean deserializeValue(
435435
baseEvent.dist = reader.nextStringOrNull();
436436
return true;
437437
case JsonKeys.BREADCRUMBS:
438-
baseEvent.breadcrumbs = reader.nextList(logger, new Breadcrumb.Deserializer());
438+
baseEvent.breadcrumbs = reader.nextListOrNull(logger, new Breadcrumb.Deserializer());
439439
return true;
440440
case JsonKeys.DEBUG_META:
441441
baseEvent.debugMeta = reader.nextOrNull(logger, new DebugMeta.Deserializer());

‎sentry/src/main/java/io/sentry/SentryEvent.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,15 @@ public static final class Deserializer implements JsonDeserializer<SentryEvent>
338338
reader.beginObject();
339339
reader.nextName(); // SentryValues.JsonKeys.VALUES
340340
event.threads =
341-
new SentryValues<>(reader.nextList(logger, new SentryThread.Deserializer()));
341+
new SentryValues<>(reader.nextListOrNull(logger, new SentryThread.Deserializer()));
342342
reader.endObject();
343343
break;
344344
case JsonKeys.EXCEPTION:
345345
reader.beginObject();
346346
reader.nextName(); // SentryValues.JsonKeys.VALUES
347347
event.exception =
348-
new SentryValues<>(reader.nextList(logger, new SentryException.Deserializer()));
348+
new SentryValues<>(
349+
reader.nextListOrNull(logger, new SentryException.Deserializer()));
349350
reader.endObject();
350351
break;
351352
case JsonKeys.LEVEL:

‎sentry/src/main/java/io/sentry/SpanContext.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,13 @@ public static final class Deserializer implements JsonDeserializer<SpanContext>
358358
}
359359

360360
if (op == null) {
361-
String message = "Missing required field \"" + JsonKeys.OP + "\"";
362-
Exception exception = new IllegalStateException(message);
363-
logger.log(SentryLevel.ERROR, message, exception);
364-
throw exception;
361+
/*
362+
This is the case for hybrid SDKs. In fact, 'op' field is not required as part of the
363+
trace context, but we utilise this class heavily also for transactions and spans, so it
364+
would be a lot of changes to make it optional and we just duct-tape it here.
365+
See doc https://develop.sentry.dev/sdk/event-payloads/contexts/#trace-context
366+
*/
367+
op = "";
365368
}
366369

367370
SpanContext spanContext = new SpanContext(traceId, spanId, op, parentSpanId, null);

‎sentry/src/main/java/io/sentry/clientreport/ClientReport.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public static final class Deserializer implements JsonDeserializer<ClientReport>
8989
break;
9090
case JsonKeys.DISCARDED_EVENTS:
9191
List<DiscardedEvent> deserializedDiscardedEvents =
92-
reader.nextList(logger, new DiscardedEvent.Deserializer());
92+
reader.nextListOrNull(logger, new DiscardedEvent.Deserializer());
9393
discardedEvents.addAll(deserializedDiscardedEvents);
9494
break;
9595
default:

‎sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public static final class Deserializer implements JsonDeserializer<ProfileMeasur
134134
break;
135135
case JsonKeys.VALUES:
136136
List<ProfileMeasurementValue> values =
137-
reader.nextList(logger, new ProfileMeasurementValue.Deserializer());
137+
reader.nextListOrNull(logger, new ProfileMeasurementValue.Deserializer());
138138
if (values != null) {
139139
data.values = values;
140140
}

0 commit comments

Comments
 (0)