Skip to content

Commit bfc5ee1

Browse files
adinauerclaude
andauthored
fix(sentry): Recover object readers after deserialization errors (#5293)
* fix(sentry): Prevent object readers from hanging on bad values Recover from deserializer failures by advancing past the broken value instead of retrying the same token forever. This keeps list and map helpers progressing and preserves later valid entries. Track nesting in JsonObjectReader so recovery also works after a partially consumed object or array. Implement skipValue in MapObjectReader and avoid consuming stack entries before type checks so failed reads do not corrupt the remaining input. Fixes GH-5278 Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Guard JsonObjectReader recovery failures Abort collection recovery gracefully when the stream is already unrecoverable instead of letting recoverValue fail from inside the original error handler. Log an explicit error and stop iterating so malformed or truncated JSON does not leave the reader in a worse state while still allowing the container to close cleanly. Refs GH-5278 Co-Authored-By: Claude <noreply@anthropic.com> * ref(sentry): Extract JsonObjectReader recovery helper Move the repeated recovery logging flow behind a dedicated helper so the collection readers stay focused on parsing and keep the guarded recovery behavior aligned across all three paths. This does not change behavior. It only removes duplicated error handling around failed value recovery. Refs GH-5278 Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Unwind MapObjectReader failures fully Recover MapObjectReader values back to their stack checkpoint so partially consumed nested objects do not leave child entries or end sentinels behind. This keeps later values readable after a deserializer fails mid-object and adds regression coverage for list, map, and map-of-lists paths that partially enter nested values before throwing. Refs GH-5278 Co-Authored-By: Claude <noreply@anthropic.com> * changelog * fix(sentry): Track container entry during JSON recovery Track recovery state per collection element so post-parse validation failures do not cause the next sibling container to be skipped. This keeps the livelock fix while preserving later valid values after a failed object deserializer. Refs GH-5278 Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Track consumed values during JSON recovery Treat recovery state as consumed when the current value has already been read, including skipValue()-driven failures. This prevents the fallback recovery step from skipping the next valid sibling after a deserializer consumes a value and then throws. Add regressions for direct list recovery and nested map-of-list recovery so consumed-value failures keep later valid entries. Refs GH-5278 Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Log unknown key recovery failures Log a second error when unknown-field recovery itself fails. This makes nextUnknown consistent with the list and map recovery paths and makes truncated payload failures easier to diagnose. Add regression coverage for unknown-key deserialization failures that also leave the stream unrecoverable. Co-Authored-By: Claude <noreply@anthropic.com> * docs(changelog): Clarify JSON recovery log levels Clarify that collection recovery emits warning logs for skipped values, while unknown-key failures and unrecoverable recovery paths emit errors. This keeps the release note aligned with the actual logging behavior from the recovery fixes. Co-Authored-By: Claude <noreply@anthropic.com> * merge changelog lines * mark consumed afterwards instead of before * test(sentry): Remove explicit timeouts from reader recovery tests Remove the explicit JUnit timeout annotations from the new JsonObjectReader and MapObjectReader recovery tests. These tests already fail deterministically and do not need per-test timeouts. Dropping the annotations keeps the new coverage aligned with the rest of the suite and avoids extra noise in the test definitions. Co-Authored-By: Claude <noreply@anthropic.com> * test(sentry): Rename mismatched MapObjectReader recovery tests Rename the partially consumed MapObjectReader recovery tests so their names match the map iteration order. MapObjectReader reads map entries in reverse insertion order from its stack. The previous names described the surviving values as coming after the failure even though they are read before it. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Mark skipped values consumed after success Move skipValue consumption tracking after the underlying Gson skip succeeds. This keeps valueConsumed aligned with the primitive reader methods and avoids marking a value as consumed before JsonReader has actually advanced past it. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Handle empty collections in MapObjectReader Gate the recovery loops on the current token instead of hasNext when reading maps and lists. MapObjectReader keeps END_OBJECT and END_ARRAY sentinels on its stack, so hasNext stays true for empty collections. Checking the token preserves empty map and empty list handling after moving nextName outside the inner recovery try-catch, and the new tests cover those cases. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Preserve list recovery in MapObjectReader Continue list recovery until the END_ARRAY sentinel instead of stopping at the next non-object token. A failed object element could be followed by an invalid primitive and then a valid object. The old loop stopped at the primitive, letting endArray pop the wrong stack entry and leaving the reader in an inconsistent state. The updated loop keeps recovering until the array is fully consumed, and the new tests cover empty collections and mixed invalid list contents. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sentry): Continue list recovery until array end Continue JsonObjectReader list deserialization until the array boundary instead of stopping when the next token is not BEGIN_OBJECT. After recovering from a failed object element, the next unread token can be a primitive. The previous loop exited early in that case and then aborted when endArray() encountered unread content. Iterating with hasNext() keeps recovery aligned with the actual array boundary. Add a regression test covering a failing object followed by a primitive and a later valid object. Fixes GH-5278 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6cf6485 commit bfc5ee1

5 files changed

Lines changed: 763 additions & 59 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
### Fixes
66

7+
- Fix `JsonObjectReader` and `MapObjectReader` hanging indefinitely when deserialization errors leave the reader in an inconsistent state ([#5293](https://github.com/getsentry/sentry-java/pull/5293))
8+
- Failed collection values are now skipped so parsing can continue
9+
- Skipped collection values emit `WARNING` logs
10+
- Unknown-key failures and unrecoverable recovery failures emit `ERROR` logs
711
- Fix ANR caused by `GestureDetectorCompat` Handler/MessageQueue lock contention in `SentryWindowCallback` ([#5138](https://github.com/getsentry/sentry-java/pull/5138))
812

913
### Internal

0 commit comments

Comments
 (0)