fix(sentry): Recover object readers after deserialization errors by adinauer · Pull Request #5293 · getsentry/sentry-java

adinauer · 2026-04-14T12:15:24Z

📜 Description

Fix JsonObjectReader and MapObjectReader so failed value deserialization no longer hangs
or corrupts subsequent reads.

The change makes both readers advance past failed values, keeps later valid entries readable,
and adds coverage for partial-consumption and truncated-input recovery paths.

💡 Motivation and Context

Some collection deserialization helpers could get stuck retrying the same value forever when a
nested deserializer threw. In other cases, partial consumption left the reader in an invalid
state and broke later values.

This fixes GH-5278.

💚 How did you test it?

Ran ./gradlew spotlessApply apiDump
Ran ./gradlew :sentry:test --tests="io.sentry.JsonObjectReaderTest" --tests="io.sentry.util.MapObjectReaderTest"
Added regression tests for skipped failed values, partially consumed nested values, and
unrecoverable truncated-input recovery

📝 Checklist

I added GH Issue ID & Linear ID
I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
I updated the wizard if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Verify the changelog wording before merge

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>

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>

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>

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>

github-actions · 2026-04-14T12:15:51Z

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).

Bug Fixes 🐛

(sentry) Recover object readers after deserialization errors by adinauer in #5293

_{🤖 This preview updates automatically when you update the PR.}

sentry · 2026-04-14T12:18:01Z

📲 Install Builds

Android

🔗 App Name	App ID	Version	Configuration
SDK Size	io.sentry.tests.size	8.39.0 (1)	release

⚙️ sentry-android Build Distribution Settings

github-actions · 2026-04-14T12:24:45Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	314.90 ms	366.78 ms	51.88 ms
Size	0 B	0 B	0 B

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`d15471f`	310.66 ms	368.19 ms	57.53 ms
`9054d65`	330.94 ms	403.24 ms	72.30 ms
`b77456b`	393.26 ms	441.10 ms	47.84 ms
`96449e8`	361.30 ms	423.39 ms	62.09 ms
`ee747ae`	357.79 ms	421.84 ms	64.05 ms
`a416a65`	295.53 ms	373.74 ms	78.21 ms
`ab8a72d`	316.24 ms	356.38 ms	40.14 ms
`1564554`	323.06 ms	336.68 ms	13.62 ms
`b3d8889`	371.69 ms	432.96 ms	61.26 ms
`d15471f`	315.20 ms	370.22 ms	55.02 ms

App size

Revision	Plain	With Sentry	Diff
`d15471f`	1.58 MiB	2.13 MiB	559.54 KiB
`9054d65`	1.58 MiB	2.29 MiB	723.38 KiB
`b77456b`	1.58 MiB	2.12 MiB	548.11 KiB
`96449e8`	1.58 MiB	2.11 MiB	539.35 KiB
`ee747ae`	1.58 MiB	2.10 MiB	530.95 KiB
`a416a65`	1.58 MiB	2.12 MiB	555.26 KiB
`ab8a72d`	1.58 MiB	2.12 MiB	551.55 KiB
`1564554`	1.58 MiB	2.20 MiB	635.33 KiB
`b3d8889`	1.58 MiB	2.10 MiB	535.06 KiB
`d15471f`	1.58 MiB	2.13 MiB	559.54 KiB

Previous results on branch: fix/jsonobjectreader-hanging

Startup times

Revision	Plain	With Sentry	Diff
`f6edf41`	314.40 ms	370.94 ms	56.53 ms
`15c2be2`	393.30 ms	474.74 ms	81.44 ms

App size

Revision	Plain	With Sentry	Diff
`f6edf41`	0 B	0 B	0 B
`15c2be2`	0 B	0 B	0 B

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>

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>

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>

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>

adinauer · 2026-04-15T14:18:45Z

@sentry review

adinauer · 2026-04-15T14:18:50Z

cursor review

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>

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>

romtsn

just one question, but LGTM otherwise!

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>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 1bcb316. Configure here.}

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>

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>

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>

adinauer and others added 4 commits April 14, 2026 13:44

changelog

98fb625

adinauer and others added 5 commits April 15, 2026 12:18

merge changelog lines

d710cca

cursor bot reviewed Apr 15, 2026

View reviewed changes

Comment thread sentry/src/main/java/io/sentry/JsonObjectReader.java

adinauer added 2 commits April 16, 2026 05:48

mark consumed afterwards instead of before

356ddda

Merge branch 'main' into fix/jsonobjectreader-hanging

558453c

adinauer marked this pull request as ready for review April 16, 2026 11:08

adinauer requested review from markushi and romtsn as code owners April 16, 2026 11:08

adinauer and others added 2 commits April 16, 2026 13:10

romtsn reviewed Apr 16, 2026

View reviewed changes

Comment thread sentry/src/main/java/io/sentry/JsonObjectReader.java Outdated

romtsn approved these changes Apr 16, 2026

View reviewed changes

sentry bot reviewed Apr 16, 2026

View reviewed changes

Comment thread sentry/src/main/java/io/sentry/util/MapObjectReader.java

cursor bot reviewed Apr 16, 2026

View reviewed changes

Comment thread sentry/src/main/java/io/sentry/util/MapObjectReader.java

adinauer and others added 2 commits April 16, 2026 15:33

adinauer and others added 2 commits April 16, 2026 15:42

Merge branch 'main' into fix/jsonobjectreader-hanging

c9d3909

Uh oh!

Conversation

adinauer commented Apr 14, 2026

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

github-actions bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Semver Impact of This PR

Bug Fixes 🐛

Uh oh!

sentry bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📲 Install Builds

Android

Uh oh!

github-actions bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/jsonobjectreader-hanging

Startup times

App size

Uh oh!

adinauer commented Apr 15, 2026

Uh oh!

adinauer commented Apr 15, 2026

Uh oh!

Uh oh!

Uh oh!

romtsn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 14, 2026 •

edited

Loading

sentry bot commented Apr 14, 2026 •

edited

Loading

github-actions bot commented Apr 14, 2026 •

edited

Loading