Skip to content

Fix the issue with changing immutable metadata structure in the contructor of ReactNativeClient - #5202

Merged
alwx merged 7 commits into
mainfrom
alwx/bug/5187
Oct 6, 2025
Merged

Fix the issue with changing immutable metadata structure in the contructor of ReactNativeClient#5202
alwx merged 7 commits into
mainfrom
alwx/bug/5187

Conversation

@alwx

@alwxalwx commented Sep 24, 2025

Copy link
Copy Markdown
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Fixes#5187

📝 Checklist

  • I added tests to verify 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.
  • All tests passing
  • No breaking changes

🔮 Next steps

@alwxalwx self-assigned this Sep 24, 2025
@alwxalwx mentioned this pull request Sep 24, 2025
Closed
@github-actions

github-actionsBot commented Sep 24, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Android (legacy) Performance metrics 🚀

PlainWith SentryDiff
Startup time441.10 ms456.54 ms15.44 ms
Size17.75 MiB19.70 MiB1.95 MiB

Baseline results on branch: main

Startup times

RevisionPlainWith SentryDiff
f70acbf+dirty373.39 ms382.81 ms9.43 ms
49ef936+dirty405.96 ms417.22 ms11.26 ms
7be1f99454.83 ms461.36 ms6.53 ms
000da7a454.46 ms445.00 ms-9.46 ms
bfe454a+dirty573.44 ms579.46 ms6.02 ms
7480abe+dirty411.60 ms405.81 ms-5.78 ms
64cd15c439.02 ms427.63 ms-11.39 ms
23080e5384.85 ms382.57 ms-2.28 ms
5526494440.84 ms448.36 ms7.52 ms
c08359e421.87 ms445.37 ms23.50 ms

App size

RevisionPlainWith SentryDiff
f70acbf+dirty17.75 MiB19.68 MiB1.94 MiB
49ef936+dirty17.75 MiB19.69 MiB1.94 MiB
7be1f9917.75 MiB20.15 MiB2.41 MiB
000da7a17.75 MiB19.68 MiB1.94 MiB
bfe454a+dirty17.75 MiB19.69 MiB1.94 MiB
7480abe+dirty17.75 MiB19.68 MiB1.94 MiB
64cd15c17.75 MiB20.15 MiB2.41 MiB
23080e517.75 MiB19.68 MiB1.94 MiB
552649417.75 MiB19.68 MiB1.93 MiB
c08359e17.75 MiB20.15 MiB2.41 MiB

Previous results on branch: alwx/bug/5187

Startup times

RevisionPlainWith SentryDiff
05bd451+dirty400.44 ms420.96 ms20.52 ms

App size

RevisionPlainWith SentryDiff
05bd451+dirty17.75 MiB19.69 MiB1.94 MiB

@github-actions

github-actionsBot commented Sep 24, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Android (new) Performance metrics 🚀

PlainWith SentryDiff
Startup time373.52 ms419.48 ms45.96 ms
Size7.15 MiB8.42 MiB1.27 MiB

Baseline results on branch: main

Startup times

RevisionPlainWith SentryDiff
a0b15d6+dirty414.33 ms448.85 ms34.52 ms
f70acbf+dirty520.12 ms558.91 ms38.79 ms
49ef936+dirty333.72 ms387.51 ms53.79 ms
bfe454a+dirty372.42 ms424.52 ms52.10 ms
7480abe+dirty363.80 ms431.34 ms67.54 ms
20d5eaa+dirty358.31 ms442.37 ms84.06 ms
c4e097a+dirty382.43 ms443.77 ms61.34 ms
7be1f99+dirty369.02 ms399.60 ms30.58 ms
64cd15c+dirty488.79 ms483.54 ms-5.24 ms
534ba8c+dirty472.35 ms537.31 ms64.96 ms

App size

RevisionPlainWith SentryDiff
a0b15d6+dirty7.15 MiB8.42 MiB1.27 MiB
f70acbf+dirty7.15 MiB8.41 MiB1.26 MiB
49ef936+dirty7.15 MiB8.42 MiB1.26 MiB
bfe454a+dirty7.15 MiB8.42 MiB1.26 MiB
7480abe+dirty7.15 MiB8.41 MiB1.26 MiB
20d5eaa+dirty7.15 MiB8.42 MiB1.27 MiB
c4e097a+dirty7.15 MiB8.41 MiB1.26 MiB
7be1f99+dirty7.15 MiB8.42 MiB1.27 MiB
64cd15c+dirty7.15 MiB8.42 MiB1.27 MiB
534ba8c+dirty7.15 MiB8.42 MiB1.27 MiB

Previous results on branch: alwx/bug/5187

Startup times

RevisionPlainWith SentryDiff
05bd451+dirty302.20 ms352.22 ms50.02 ms

App size

RevisionPlainWith SentryDiff
05bd451+dirty7.15 MiB8.42 MiB1.26 MiB

@github-actions

github-actionsBot commented Sep 24, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

iOS (legacy) Performance metrics 🚀

PlainWith SentryDiff
Startup time1216.77 ms1236.21 ms19.45 ms
Size2.63 MiB3.98 MiB1.35 MiB

Baseline results on branch: main

Startup times

RevisionPlainWith SentryDiff
ec14be7+dirty1234.64 ms1245.54 ms10.90 ms
d751a5d+dirty1215.57 ms1220.56 ms4.99 ms
23080e5+dirty1216.02 ms1224.94 ms8.91 ms
64cd15c+dirty1216.31 ms1214.04 ms-2.26 ms
77061ed+dirty1233.16 ms1234.88 ms1.71 ms
ba75c7c+dirty1235.86 ms1226.45 ms-9.41 ms
95aaf8a+dirty1234.78 ms1241.94 ms7.16 ms
98f632c+dirty1236.40 ms1241.62 ms5.22 ms
534ba8c+dirty1230.22 ms1231.18 ms0.96 ms
a31630c+dirty1229.09 ms1230.94 ms1.85 ms

App size

RevisionPlainWith SentryDiff
ec14be7+dirty2.63 MiB3.98 MiB1.34 MiB
d751a5d+dirty2.63 MiB3.98 MiB1.34 MiB
23080e5+dirty2.63 MiB3.91 MiB1.28 MiB
64cd15c+dirty2.63 MiB3.81 MiB1.18 MiB
77061ed+dirty2.63 MiB3.98 MiB1.34 MiB
ba75c7c+dirty2.63 MiB3.81 MiB1.18 MiB
95aaf8a+dirty2.63 MiB3.87 MiB1.24 MiB
98f632c+dirty2.63 MiB3.81 MiB1.18 MiB
534ba8c+dirty2.63 MiB3.81 MiB1.18 MiB
a31630c+dirty2.63 MiB3.98 MiB1.34 MiB

Previous results on branch: alwx/bug/5187

Startup times

RevisionPlainWith SentryDiff
05bd451+dirty1219.96 ms1236.88 ms16.92 ms

App size

RevisionPlainWith SentryDiff
05bd451+dirty2.63 MiB3.98 MiB1.34 MiB

@github-actions

github-actionsBot commented Sep 24, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

iOS (new) Performance metrics 🚀

PlainWith SentryDiff
Startup time1239.33 ms1234.75 ms-4.58 ms
Size3.19 MiB4.55 MiB1.36 MiB

Baseline results on branch: main

Startup times

RevisionPlainWith SentryDiff
ec14be7+dirty1229.62 ms1230.53 ms0.91 ms
d751a5d+dirty1212.22 ms1217.94 ms5.71 ms
23080e5+dirty1221.39 ms1222.08 ms0.70 ms
64cd15c+dirty1213.50 ms1223.54 ms10.04 ms
77061ed+dirty1210.77 ms1218.45 ms7.68 ms
ba75c7c+dirty1236.14 ms1240.69 ms4.55 ms
95aaf8a+dirty1206.83 ms1213.65 ms6.81 ms
98f632c+dirty1221.38 ms1229.26 ms7.88 ms
534ba8c+dirty1225.00 ms1237.43 ms12.43 ms
a31630c+dirty1241.32 ms1226.98 ms-14.34 ms

App size

RevisionPlainWith SentryDiff
ec14be7+dirty3.19 MiB4.54 MiB1.36 MiB
d751a5d+dirty3.19 MiB4.54 MiB1.36 MiB
23080e5+dirty3.19 MiB4.48 MiB1.29 MiB
64cd15c+dirty3.19 MiB4.38 MiB1.19 MiB
77061ed+dirty3.19 MiB4.54 MiB1.36 MiB
ba75c7c+dirty3.19 MiB4.38 MiB1.19 MiB
95aaf8a+dirty3.19 MiB4.44 MiB1.25 MiB
98f632c+dirty3.19 MiB4.38 MiB1.19 MiB
534ba8c+dirty3.19 MiB4.38 MiB1.19 MiB
a31630c+dirty3.19 MiB4.54 MiB1.36 MiB

Previous results on branch: alwx/bug/5187

Startup times

RevisionPlainWith SentryDiff
05bd451+dirty1223.57 ms1237.29 ms13.72 ms

App size

RevisionPlainWith SentryDiff
05bd451+dirty3.19 MiB4.54 MiB1.36 MiB

@alwxalwx changed the title Fix the issue with changing immutable metadata structure in the contructor of ReactNativeClientWIP: Fix the issue with changing immutable metadata structure in the contructor of ReactNativeClientSep 25, 2025
@alwx
alwx marked this pull request as draft September 25, 2025 06:53
lucas-zimerman
lucas-zimerman reviewed Sep 26, 2025
View reviewed changes
Comment threadpackages/core/src/js/client.ts
...options._metadata?.sdk?.settings,
},
},
};

@lucas-zimermanlucas-zimermanSep 26, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The changes follows the new implementation getsentry/sentry-javascript#17364

@alwxalwxSep 30, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, but the issue here is that the metadata structure is immutable so instead of modifying it we can construct a new one, and that's what I'm doing in this PR. Other than that, the logic stays pretty much the same.

I've also updated tests to reflect that change and indiciate the expected behaviour which is the following:

  • If sendDefaultPii is true, Sentry will infer the IP address of users' devices to events (errors, traces, replays, etc) in all browser-based SDKs.
  • If sendDefaultPii is false or not set, Sentry will not infer or collect IP address data.

@alwx

alwx commented Sep 30, 2025

Copy link
Copy Markdown
ContributorAuthor

I've updated this PR and updated some tests that were not correct — all those tests were checking if infer_ip is set to never which should happen only if setDefaultPii is set to false.
There are no new tests added since there are already tests that check for infer_ip value. Those are the following:

  • does not add ip_address {{auto}} to undefined user if sendDefaultPii is false
  • doesn't change infer_ip if the ip_address is set to undefined (I've updated the name)
  • doesn't change infer_ip if the user is not set (I've updated the name)
  • doesn't change infer_ip if the event is empty

@alwxalwx changed the title WIP: Fix the issue with changing immutable metadata structure in the contructor of ReactNativeClient Fix the issue with changing immutable metadata structure in the contructor of ReactNativeClientSep 30, 2025
@alwx
alwx marked this pull request as ready for review September 30, 2025 08:44
antonis
antonis reviewed Sep 30, 2025
View reviewed changes

@antonisantonis 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.

The change LGTM and handle the reported issue. My understanding is that it also does not break the sendDefaultPii fix. That said, I'm leaving the final approval to @lucas-zimerman who has more context on that patch.
We should also add a changelog entry since this is user facing and fixes an issue.

lucas-zimerman
lucas-zimerman reviewed Sep 30, 2025
View reviewed changes
Comment threadpackages/core/test/client.test.ts
lucas-zimerman
lucas-zimerman reviewed Sep 30, 2025
View reviewed changes
Comment threadpackages/core/test/client.test.ts
lucas-zimerman
lucas-zimerman reviewed Sep 30, 2025
View reviewed changes
Comment threadpackages/core/test/client.test.ts
lucas-zimerman
lucas-zimerman approved these changes Sep 30, 2025
View reviewed changes

@lucas-zimermanlucas-zimerman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should follow the quote pattern as it's used on the remaining tests.
Also after adding a changelog, LGTM!

@alwx
alwx requested a review from antonisOctober 1, 2025 12:08
antonis
antonis approved these changes Oct 1, 2025
View reviewed changes

@antonisantonis 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.

LGTM 🎸
Thank you for fixing this @alwx 🙇

@antonis

antonis commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

If this is not a CI issue 😓 there seems to be an issue with the linter (ERROR: "lint:prettier" exited with 1.) which might be fixed with a yarn fix.

@lucas-zimerman

lucas-zimerman commented Oct 1, 2025

Copy link
Copy Markdown
Collaborator

If this is not a CI issue 😓 there seems to be an issue with the linter (ERROR: "lint:prettier" exited with 1.) which might be fixed with a yarn fix.

probably related to

 Error while parsing /home/runner/work/sentry-react-native/sentry-react-native/packages/core/node_modules/react-native/Libraries/Utilities/codegenNativeComponent.js
Line 19, column 26: Property or signature expected.
`parseForESLint` from parser `/home/runner/work/sentry-react-native/sentry-react-native/packages/core/node_modules/@typescript-eslint/parser/dist/index.js` is invalid and will just be ignored

@alwx
alwx merged commit 170d5ea into mainOct 6, 2025
65 checks passed
@alwx
alwx deleted the alwx/bug/5187 branch October 6, 2025 09:33
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antonisantonisantonis approved these changes
@lucas-zimermanlucas-zimermanlucas-zimerman approved these changes

Assignees

@alwxalwx

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@alwx@antonis@lucas-zimerman