Skip to content

fix: Support geo in SentryGeo initWithDictionary#8026

Merged
antonis merged 2 commits into
mainfrom
fix/geo-init-with-dictionary
Jun 9, 2026
Merged

fix: Support geo in SentryGeo initWithDictionary#8026
antonis merged 2 commits into
mainfrom
fix/geo-init-with-dictionary

Conversation

@antonis

@antonis antonis commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

📜 Description

Add initWithDictionary: to SentryGeo (via a private header) and handle the "geo" key in SentryUser.initWithDictionary: so that geo data is properly deserialized into the geo property instead of landing in the unknown dictionary.

Previously, passing a dictionary with a "geo" key to SentryUser.initWithDictionary: would store it in unknown. During serialization, unknown keys are written on top of the serialized dict, overwriting the properly-set self.geo property — causing duplicated or incorrect geo data.

💡 Motivation and Context

Hybrid SDKs (e.g. React Native) pass user dictionaries that include "geo". Without this fix, they need to manually strip the "geo" key before calling initWithDictionary: and set it separately — a workaround that was identified in getsentry/sentry-react-native#6261 (comment).

💚 How did you test it?

  • Added test for initWithDictionary: with geo fields
  • Added round-trip test (dict → initWithDictionary → serialize) to verify geo survives serialization without leaking into unknown
  • All 14 SentryUserTests pass

📝 Checklist

  • 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.
  • If I added a new public API, I also added it to the SentryObjC wrapper.

#skip-changelog

Add initWithDictionary: to SentryGeo and handle the
"geo" key in SentryUser.initWithDictionary: so that
geo data is properly deserialized instead of landing
in the unknown dict and overwriting the serialized geo.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit aef9575. Configure here.

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.612%. Comparing base (7087caa) to head (62f8b36).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #8026       +/-   ##
=============================================
+ Coverage   86.603%   86.612%   +0.009%     
=============================================
  Files          550       550               
  Lines        31695     31709       +14     
  Branches     13043     13056       +13     
=============================================
+ Hits         27449     27464       +15     
- Misses        4196      4198        +2     
+ Partials        50        47        -3     
Files with missing lines Coverage Δ
Sources/Sentry/SentryGeo.m 94.000% <100.000%> (+1.692%) ⬆️
Sources/Sentry/SentryUser.m 95.918% <100.000%> (+0.085%) ⬆️

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7087caa...62f8b36. Read the comment docs.

@sentry

sentry Bot commented Jun 9, 2026

Copy link
Copy Markdown

📲 Install Builds

iOS

🔗 App Name App ID Version Configuration
SDK-Size io.sentry.sample.SDK-Size 9.16.1 (1) Release

⚙️ sentry-cocoa Build Distribution Settings

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1233.27 ms 1264.67 ms 31.39 ms
Size 24.14 KiB 1.17 MiB 1.15 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dd70dad 1233.87 ms 1269.91 ms 36.04 ms
6b08499 1216.67 ms 1247.76 ms 31.08 ms
eaa30de 1232.96 ms 1265.43 ms 32.47 ms
69745fc 1226.70 ms 1259.72 ms 33.02 ms
92fada5 1217.60 ms 1252.89 ms 35.30 ms
1887b2e 1218.51 ms 1243.06 ms 24.55 ms
2509eff 1219.50 ms 1253.57 ms 34.07 ms
e0946cf 1233.33 ms 1258.57 ms 25.24 ms
9c19a06 1217.77 ms 1248.98 ms 31.21 ms
07d6099 1224.06 ms 1247.13 ms 23.06 ms

App size

Revision Plain With Sentry Diff
dd70dad 24.14 KiB 1.17 MiB 1.15 MiB
6b08499 24.14 KiB 1.15 MiB 1.13 MiB
eaa30de 24.14 KiB 1.17 MiB 1.15 MiB
69745fc 24.14 KiB 1.15 MiB 1.13 MiB
92fada5 24.14 KiB 1.17 MiB 1.15 MiB
1887b2e 24.14 KiB 1.15 MiB 1.13 MiB
2509eff 24.14 KiB 1.17 MiB 1.15 MiB
e0946cf 24.14 KiB 1.16 MiB 1.13 MiB
9c19a06 24.14 KiB 1.16 MiB 1.13 MiB
07d6099 24.14 KiB 1.17 MiB 1.15 MiB

Previous results on branch: fix/geo-init-with-dictionary

Startup times

Revision Plain With Sentry Diff
85d56f0 1238.32 ms 1273.45 ms 35.13 ms

App size

Revision Plain With Sentry Diff
85d56f0 24.14 KiB 1.17 MiB 1.15 MiB

@antonis antonis removed the ready-to-merge Use this label to trigger all PR workflows label Jun 9, 2026
@antonis antonis marked this pull request as ready for review June 9, 2026 12:37

@itaybre itaybre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Almost LGTM, just one comment

Comment thread Sources/Sentry/SentryGeo.m
Cover all properties, partial input, empty dict, wrong
types, and serialize round-trip.
@antonis antonis added the ready-to-merge Use this label to trigger all PR workflows label Jun 9, 2026
@antonis antonis requested a review from itaybre June 9, 2026 13:19
@antonis antonis removed the ready-to-merge Use this label to trigger all PR workflows label Jun 9, 2026

@philprime philprime left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@antonis antonis merged commit 1936da3 into main Jun 9, 2026
304 of 317 checks passed
@antonis antonis deleted the fix/geo-init-with-dictionary branch June 9, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants